Implement path encoding and decoding
sdbus has methods for encoding and decoding paths in a lossless way.
This commit adds those methods to object_path in such a way that those
functions are called implicitly when building strings, the hope being
that any API choosing to use object_path need not write its own escaping
functions, and said escaping will happen implicitly.
The explicit changes to make this happen are.
1. filename() now calls into sd_bus_path_decode, rather than writing its
own method. This means that the strings returned from the method now no
longer need decoding explicitly, and can largely be used as-is. For
paths that did not require escaping, the behavior should be identical.
For paths that did require escaping, this method will now return the raw
version of the path. Existing usages should remain the same, and impact
is unlikely, given that most sdbusplus users don't explicitly escape
strings, and filename() is relatively new.
2. a operator/ method is added to the object_path object. Similar to
operator/ on std::filesystem::path, this allows generating a new
object_path from an existing one, by adding a new leaf node to the path.
Functionally this allows nearly any value to be escaped into a dbus
path. An example of a common usage of this might be
object_path myPath = "/";
object_path myNewPath = myPath / "Foo-bar";
The above line will implicitly escape the - in the name. Overloads for
both const std::string& and const char* have been added.
One motivating example where the above would be useful is to remove the
escapePathForDbus method in bmcweb.
https://github.com/openbmc/bmcweb/blob/88b3dd12851cd7bdd4b5c065ba99f40feafb775e/include/dbus_utility.hpp#L47
and the similarly named method in dbus-sensors
https://github.com/openbmc/dbus-sensors/blob/6cb732a31b7664089124b00e806311768bc24a87/src/SensorPaths.cpp#L49
both written by the same author (not me), and centralize them in
sdbusplus where they belong.
Functionally this commit also includes a new utility/memory.hpp, that
allows for C-style RAII using freep, mfree and the _cleanup_free_
attributes. These were largely copied as-is from alloc-util.h in
systemd on latest patchset recommendations.
Tested:
Unit tests have been updated to include all new code paths, and some
common items that might need escaped. Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If77b8bc5f75709b59353570a9c14383cac44dcd4
diff --git a/include/sdbusplus/message/native_types.hpp b/include/sdbusplus/message/native_types.hpp
index fe695c6..f93d775 100644
--- a/include/sdbusplus/message/native_types.hpp
+++ b/include/sdbusplus/message/native_types.hpp
@@ -1,5 +1,7 @@
#pragma once
+#include <sdbusplus/utility/memory.hpp>
+
#include <string>
namespace sdbusplus
@@ -76,6 +78,7 @@
return l < r.str;
}
};
+
/** Simple wrapper class for std::string to allow conversion to and from an
* alternative typename. */
struct string_path_wrapper
@@ -143,18 +146,15 @@
std::string filename() const
{
- auto index = str.rfind('/');
- if (index == std::string::npos)
+ std::string parent = parent_path();
+ _cleanup_free_ char* out = nullptr;
+ int r = sd_bus_path_decode(str.c_str(), parent.c_str(), &out);
+ if (r <= 0)
{
return "";
}
- index++;
- if (index >= str.size())
- {
- return "";
- }
-
- return str.substr(index);
+ std::string ret(out);
+ return ret;
}
string_path_wrapper parent_path() const
@@ -171,6 +171,24 @@
return str.substr(0, index);
}
+
+ string_path_wrapper operator/(const std::string& extId)
+ {
+ return this->operator/(extId.c_str());
+ }
+
+ string_path_wrapper operator/(const char* extId)
+ {
+ string_path_wrapper out;
+ _cleanup_free_ char* encOut = nullptr;
+ int ret = sd_bus_path_encode(str.c_str(), extId, &encOut);
+ if (ret < 0)
+ {
+ return out;
+ }
+ out.str = encOut;
+ return out;
+ }
};
/** Typename for sdbus SIGNATURE types. */
diff --git a/include/sdbusplus/utility/memory.hpp b/include/sdbusplus/utility/memory.hpp
new file mode 100644
index 0000000..49dc999
--- /dev/null
+++ b/include/sdbusplus/utility/memory.hpp
@@ -0,0 +1,16 @@
+#pragma once
+
+#include <cstdlib>
+
+static inline void* mfree(void* memory)
+{
+ free(memory);
+ return NULL;
+}
+
+static inline void freep(void* p)
+{
+ *(void**)p = mfree(*(void**)p);
+}
+#define _cleanup_free_ _cleanup_(freep)
+#define _cleanup_(x) __attribute__((__cleanup__(x)))
\ No newline at end of file
diff --git a/test/message/types.cpp b/test/message/types.cpp
index 9f82f0a..b47d39b 100644
--- a/test/message/types.cpp
+++ b/test/message/types.cpp
@@ -41,7 +41,7 @@
ASSERT_EQ(dbus_string(sdbusplus::message::object_path("/asdf")), "o");
}
-TEST(MessageTypes, ObjectPathLeaf)
+TEST(MessageTypes, ObjectPathFilename)
{
ASSERT_EQ(sdbusplus::message::object_path("/abc/def").filename(), "def");
ASSERT_EQ(sdbusplus::message::object_path("/abc/").filename(), "");
@@ -49,6 +49,9 @@
ASSERT_EQ(sdbusplus::message::object_path("/").filename(), "");
ASSERT_EQ(sdbusplus::message::object_path("").filename(), "");
ASSERT_EQ(sdbusplus::message::object_path("abc").filename(), "");
+ ASSERT_EQ(sdbusplus::message::object_path("/_2d").filename(), "-");
+ ASSERT_EQ(sdbusplus::message::object_path("/_20").filename(), " ");
+ ASSERT_EQ(sdbusplus::message::object_path("/_2F").filename(), "/");
}
TEST(MessageTypes, ObjectPathParent)
@@ -63,6 +66,24 @@
sdbusplus::message::object_path("/"));
}
+TEST(MessageTypes, ObjectPathOperatorSlash)
+{
+ ASSERT_EQ(sdbusplus::message::object_path("/") / "abc",
+ sdbusplus::message::object_path("/abc"));
+ ASSERT_EQ(sdbusplus::message::object_path("/abc") / "def",
+ sdbusplus::message::object_path("/abc/def"));
+ ASSERT_EQ(sdbusplus::message::object_path("/abc") / "-",
+ sdbusplus::message::object_path("/abc/_2d"));
+ ASSERT_EQ(sdbusplus::message::object_path("/abc") / " ",
+ sdbusplus::message::object_path("/abc/_20"));
+ ASSERT_EQ(sdbusplus::message::object_path("/abc") / "/",
+ sdbusplus::message::object_path("/abc/_2f"));
+
+ // Test the std::string overload. This is largely just for coverage
+ ASSERT_EQ(sdbusplus::message::object_path("/") / std::string("abc"),
+ sdbusplus::message::object_path("/abc"));
+}
+
TEST(MessageTypes, Signature)
{
ASSERT_EQ(dbus_string(sdbusplus::message::signature("sss")), "g");