Fix bug in name escaping for PSUSensor
PSU sensor has a bug where if the name includes any invalid characters,
it crashes when attempting to launch. This is a problem, and very much
not expected behavior.
The root cause of this is that PSU combine event is creating a dbus API
path from the object name, but neglects to escape it properly.
The previous method to escape this was inlined inside sensor.hpp, so
that has been promoted to a new utility function, escapeForDbus which
can return an escaped name string to use in paths.
Through the course of adding this to the sensors namespace and including
it, it turns out that "sensors" is overloaded, and including it causes
both compile time errors from the overloaded name, and link time errors
from the lack of "inline" on the utility methods. This commit also as a
matter of cleanup moves everything in SensorPaths.hpp into an
alternative file, SensorPaths.cpp, and includes that file in sensor
utils.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id9ed915e7e0e13ee7bded67a785ab0fa58982ca8
diff --git a/include/SensorPaths.hpp b/include/SensorPaths.hpp
index 7becd18..e8ed8fa 100644
--- a/include/SensorPaths.hpp
+++ b/include/SensorPaths.hpp
@@ -1,55 +1,18 @@
#pragma once
#include <cstring>
+#include <regex>
#include <string>
-namespace sensors
+namespace sensor_paths
{
-// This is the leading prefix of the object path for sensors.
-// The full path is /xyz/openbmc_project/sensors/<Measure>/<Name>
-// Note C++20 would allow this to be "constexpr std::string"
-const char* objectPathPrefix = "/xyz/openbmc_project/sensors/";
-
// This is an allowlist of the units a sensor can measure. Should be in sync
// with
// phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Sensor/Value.interface.yaml#L35
-std::string getPathForUnits(const std::string& units)
-{
- if (units == "DegreesC")
- {
- return "temperature";
- }
- if (units == "RPMS")
- {
- return "fan_tach";
- }
- if (units == "Volts")
- {
- return "voltage";
- }
- if (units == "Meters")
- {
- return "altitude";
- }
- if (units == "Amperes")
- {
- return "current";
- }
- if (units == "Watts")
- {
- return "power";
- }
- if (units == "Joules")
- {
- return "energy";
- }
- if (units == "Percent")
- {
- return "Utilization";
- }
- return "";
-}
+std::string getPathForUnits(const std::string& units);
-} // namespace sensors
+std::string escapePathForDbus(const std::string& name);
+
+} // namespace sensor_paths
diff --git a/include/sensor.hpp b/include/sensor.hpp
index 115b8fa..0f90118 100644
--- a/include/sensor.hpp
+++ b/include/sensor.hpp
@@ -1,5 +1,6 @@
#pragma once
+#include <SensorPaths.hpp>
#include <Thresholds.hpp>
#include <Utils.hpp>
#include <sdbusplus/asio/object_server.hpp>
@@ -41,7 +42,7 @@
const double max, const double min,
std::shared_ptr<sdbusplus::asio::connection>& conn,
PowerState readState = PowerState::always) :
- name(std::regex_replace(name, std::regex("[^a-zA-Z0-9_/]+"), "_")),
+ name(sensor_paths::escapePathForDbus(name)),
configurationPath(configurationPath), objectType(objectType),
maxValue(max), minValue(min), thresholds(std::move(thresholdData)),
hysteresisTrigger((max - min) * 0.01),
diff --git a/meson.build b/meson.build
index 3932a14..13d9d51 100644
--- a/meson.build
+++ b/meson.build
@@ -41,7 +41,7 @@
utils_a = static_library(
'utils_a',
- 'src/Utils.cpp',
+ ['src/Utils.cpp', 'src/SensorPaths.cpp'],
implicit_include_directories: false,
include_directories: 'include',
)
diff --git a/src/ExternalSensor.cpp b/src/ExternalSensor.cpp
index f867151..c19ec40 100644
--- a/src/ExternalSensor.cpp
+++ b/src/ExternalSensor.cpp
@@ -36,12 +36,12 @@
// The caller must specify what physical characteristic
// an external sensor is expected to be measuring, such as temperature,
// as, unlike others, this is not implied by device type name.
- std::string dbusPath = sensors::getPathForUnits(sensorUnits);
+ std::string dbusPath = sensor_paths::getPathForUnits(sensorUnits);
if (dbusPath.empty())
{
throw std::runtime_error("Units not in allow list");
}
- std::string objectPath = sensors::objectPathPrefix;
+ std::string objectPath = "/xyz/openbmc_project/sensors/";
objectPath += dbusPath;
objectPath += '/';
objectPath += sensorName;
diff --git a/src/PSUEvent.cpp b/src/PSUEvent.cpp
index 375c24d..9e73df5 100644
--- a/src/PSUEvent.cpp
+++ b/src/PSUEvent.cpp
@@ -17,6 +17,7 @@
#include <systemd/sd-journal.h>
#include <PSUEvent.hpp>
+#include <SensorPaths.hpp>
#include <boost/asio/io_service.hpp>
#include <boost/asio/read_until.hpp>
#include <boost/container/flat_map.hpp>
@@ -44,8 +45,9 @@
const std::string& combineEventName) :
objServer(objectServer)
{
+ std::string psuNameEscaped = sensor_paths::escapePathForDbus(psuName);
eventInterface = objServer.add_interface(
- "/xyz/openbmc_project/State/Decorator/" + psuName + "_" +
+ "/xyz/openbmc_project/State/Decorator/" + psuNameEscaped + "_" +
combineEventName,
"xyz.openbmc_project.State.Decorator.OperationalStatus");
eventInterface->register_property("functional", true);
diff --git a/src/SensorPaths.cpp b/src/SensorPaths.cpp
new file mode 100644
index 0000000..f10e60b
--- /dev/null
+++ b/src/SensorPaths.cpp
@@ -0,0 +1,54 @@
+#include <cstring>
+#include <regex>
+#include <string>
+
+namespace sensor_paths
+{
+
+// This is an allowlist of the units a sensor can measure. Should be in sync
+// with
+// phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Sensor/Value.interface.yaml#L35
+
+std::string getPathForUnits(const std::string& units)
+{
+ if (units == "DegreesC")
+ {
+ return "temperature";
+ }
+ if (units == "RPMS")
+ {
+ return "fan_tach";
+ }
+ if (units == "Volts")
+ {
+ return "voltage";
+ }
+ if (units == "Meters")
+ {
+ return "altitude";
+ }
+ if (units == "Amperes")
+ {
+ return "current";
+ }
+ if (units == "Watts")
+ {
+ return "power";
+ }
+ if (units == "Joules")
+ {
+ return "energy";
+ }
+ if (units == "Percent")
+ {
+ return "Utilization";
+ }
+ return "";
+}
+
+std::string escapePathForDbus(const std::string& name)
+{
+ return std::regex_replace(name, std::regex("[^a-zA-Z0-9_/]+"), "_");
+}
+
+} // namespace sensor_paths