dbus-sensors: utils: Utility to get device bus & addr from dev name.
This is applicable to all the services which rely on "bus-addr" fmt.
1. FanSensor
2. HwmonTempSensor
3. IntelCPUSensor
4. PSUSensor
In addition this would also fix Fansensor Daemon crashes due to
stoi() exceptions weren't caught earlier.
For example: In a FanSensor Daemon -
Device of f0103000.pwm-fan-controller would be classified as i2cfan
based on new way of defining fan type.
Hence when we parse string for bus-addr, bus=f0103000.pwm and
addr=fan-controller for which stoi() would crash.
This would be true for all non I2c devices defaulting to I2cFan type.
Solution is to use 'std::from_chars' which handles under/overflow
properly. Centralizing this now in Utils would also allow us to manage
this appropriatelty across various services.
Tested:
Tested sanity of all daemons in the system and they work as expected.
Change-Id: I546e967abae7c0fb9fca645867e3037046037647
Signed-off-by: Akshit Shah <shahakshit@google.com>
diff --git a/src/FanMain.cpp b/src/FanMain.cpp
index 9c8a738..7cfb533 100644
--- a/src/FanMain.cpp
+++ b/src/FanMain.cpp
@@ -332,20 +332,15 @@
}
if (fanType == FanTypes::i2c)
{
- size_t bus = 0;
- size_t address = 0;
-
- std::string link =
+ std::string deviceName =
fs::read_symlink(directory / "device").filename();
- size_t findDash = link.find('-');
- if (findDash == std::string::npos ||
- link.size() <= findDash + 1)
+ size_t bus = 0;
+ size_t addr = 0;
+ if (!getDeviceBusAddr(deviceName, bus, addr))
{
- std::cerr << "Error finding device from symlink";
+ continue;
}
- bus = std::stoi(link.substr(0, findDash));
- address = std::stoi(link.substr(findDash + 1), nullptr, 16);
auto findBus = baseConfiguration->second.find("Bus");
auto findAddress =
@@ -362,7 +357,7 @@
unsigned int configAddress = std::visit(
VariantToUnsignedIntVisitor(), findAddress->second);
- if (configBus == bus && configAddress == address)
+ if (configBus == bus && configAddress == addr)
{
sensorData = &cfgData;
break;
diff --git a/src/HwmonTempMain.cpp b/src/HwmonTempMain.cpp
index a85d13a..7cc758c 100644
--- a/src/HwmonTempMain.cpp
+++ b/src/HwmonTempMain.cpp
@@ -299,27 +299,10 @@
device = directory / "device";
deviceName = fs::canonical(device).stem();
}
- auto findHyphen = deviceName.find('-');
- if (findHyphen == std::string::npos)
- {
- std::cerr << "found bad device " << deviceName << "\n";
- continue;
- }
- std::string busStr = deviceName.substr(0, findHyphen);
- std::string addrStr = deviceName.substr(findHyphen + 1);
- uint64_t bus = 0;
- uint64_t addr = 0;
- std::from_chars_result res{};
- res = std::from_chars(busStr.data(), busStr.data() + busStr.size(),
- bus);
- if (res.ec != std::errc{})
- {
- continue;
- }
- res = std::from_chars(addrStr.data(),
- addrStr.data() + addrStr.size(), addr, 16);
- if (res.ec != std::errc{})
+ size_t bus = 0;
+ size_t addr = 0;
+ if (!getDeviceBusAddr(deviceName, bus, addr))
{
continue;
}
diff --git a/src/IntelCPUSensorMain.cpp b/src/IntelCPUSensorMain.cpp
index ba36e41..005ed7e 100644
--- a/src/IntelCPUSensorMain.cpp
+++ b/src/IntelCPUSensorMain.cpp
@@ -195,23 +195,10 @@
fs::path::iterator it = hwmonNamePath.begin();
std::advance(it, 6); // pick the 6th part for a PECI client device name
std::string deviceName = *it;
- auto findHyphen = deviceName.find('-');
- if (findHyphen == std::string::npos)
- {
- std::cerr << "found bad device " << deviceName << "\n";
- continue;
- }
- std::string busStr = deviceName.substr(0, findHyphen);
- std::string addrStr = deviceName.substr(findHyphen + 1);
size_t bus = 0;
size_t addr = 0;
- try
- {
- bus = std::stoi(busStr);
- addr = std::stoi(addrStr, nullptr, 16);
- }
- catch (const std::invalid_argument&)
+ if (!getDeviceBusAddr(deviceName, bus, addr))
{
continue;
}
diff --git a/src/PSUSensorMain.cpp b/src/PSUSensorMain.cpp
index aceb094..ab439b8 100644
--- a/src/PSUSensorMain.cpp
+++ b/src/PSUSensorMain.cpp
@@ -368,27 +368,10 @@
devType = DevTypes::IIO;
}
- auto findHyphen = deviceName.find('-');
- if (findHyphen == std::string::npos)
- {
- std::cerr << "found bad device" << deviceName << "\n";
- continue;
- }
- std::string busStr = deviceName.substr(0, findHyphen);
- std::string addrStr = deviceName.substr(findHyphen + 1);
-
size_t bus = 0;
size_t addr = 0;
-
- try
+ if (!getDeviceBusAddr(deviceName, bus, addr))
{
- bus = std::stoi(busStr);
- addr = std::stoi(addrStr, nullptr, 16);
- }
- catch (const std::invalid_argument&)
- {
- std::cerr << "Error parsing bus " << busStr << " addr " << addrStr
- << "\n";
continue;
}
diff --git a/src/Utils.cpp b/src/Utils.cpp
index d659d37..ef70c1b 100644
--- a/src/Utils.cpp
+++ b/src/Utils.cpp
@@ -850,3 +850,31 @@
}
return setupPropertiesChangedMatches(bus, {types}, handler);
}
+
+bool getDeviceBusAddr(const std::string& deviceName, size_t& bus, size_t& addr)
+{
+ auto findHyphen = deviceName.find('-');
+ if (findHyphen == std::string::npos)
+ {
+ std::cerr << "found bad device " << deviceName << "\n";
+ return false;
+ }
+ std::string busStr = deviceName.substr(0, findHyphen);
+ std::string addrStr = deviceName.substr(findHyphen + 1);
+
+ std::from_chars_result res{};
+ res = std::from_chars(&*busStr.begin(), &*busStr.end(), bus);
+ if (res.ec != std::errc{} || res.ptr != &*busStr.end())
+ {
+ std::cerr << "Error finding bus for " << deviceName << "\n";
+ return false;
+ }
+ res = std::from_chars(&*addrStr.begin(), &*addrStr.end(), addr, 16);
+ if (res.ec != std::errc{} || res.ptr != &*addrStr.end())
+ {
+ std::cerr << "Error finding addr for " << deviceName << "\n";
+ return false;
+ }
+
+ return true;
+}
diff --git a/src/Utils.hpp b/src/Utils.hpp
index 0373563..9c8cdcc 100644
--- a/src/Utils.hpp
+++ b/src/Utils.hpp
@@ -387,3 +387,4 @@
setupPropertiesChangedMatches(
sdbusplus::asio::connection& bus, std::span<const char* const> types,
const std::function<void(sdbusplus::message_t&)>& handler);
+bool getDeviceBusAddr(const std::string& deviceName, size_t& bus, size_t& addr);
diff --git a/tests/test_Utils.cpp b/tests/test_Utils.cpp
index 8a52699..67fc3a5 100644
--- a/tests/test_Utils.cpp
+++ b/tests/test_Utils.cpp
@@ -173,3 +173,53 @@
EXPECT_TRUE(ret);
EXPECT_EQ(foundPaths.size(), 3U);
}
+
+TEST(GetDeviceBusAddrTest, DevNameInvalid)
+{
+ size_t bus = 0;
+ size_t addr = 0;
+ std::string devName;
+
+ auto ret = getDeviceBusAddr(devName, bus, addr);
+ EXPECT_FALSE(ret);
+
+ devName = "NoHyphen";
+ ret = getDeviceBusAddr(devName, bus, addr);
+ EXPECT_FALSE(ret);
+
+ devName = "pwm-fan";
+ ret = getDeviceBusAddr(devName, bus, addr);
+ EXPECT_FALSE(ret);
+}
+
+TEST(GetDeviceBusAddrTest, BusInvalid)
+{
+ size_t bus = 0;
+ size_t addr = 0;
+ std::string devName = "FF-00FF";
+
+ auto ret = getDeviceBusAddr(devName, bus, addr);
+ EXPECT_FALSE(ret);
+}
+
+TEST(GetDeviceBusAddrTest, AddrInvalid)
+{
+ size_t bus = 0;
+ size_t addr = 0;
+ std::string devName = "12-fan";
+
+ auto ret = getDeviceBusAddr(devName, bus, addr);
+ EXPECT_FALSE(ret);
+}
+
+TEST(GetDeviceBusAddrTest, AllValid)
+{
+ size_t bus = 0;
+ size_t addr = 0;
+ std::string devName = "12-00af";
+
+ auto ret = getDeviceBusAddr(devName, bus, addr);
+ EXPECT_TRUE(ret);
+ EXPECT_EQ(bus, 12);
+ EXPECT_EQ(addr, 0xaf);
+}