Add filename() and parent_path() methods to object_path
In practice, one very common operation done on a dbus path is to get the
final member name in the dbus path, to use as unique IDs, or to pass
items to public interfaces. This tends to lead to something like:
size_t pos = object_path.str.rfind('/');
if(pos == std::string::npos){
// handle error
}
pos++;
if (pos >= object_path.str.size()){
// handle error
}
std::string name = object_path.str.substr(pos);
As an aside, what I've written above is one of several "right" ways to
do it, and is among many other wrong ways that I've seen people try to
check in. The goal of this patchset is to add the above code as a
method within object_path, to help people to use it, and to avoid using
object_path.str, which ideally would be a private member of that class.
Functionally, accomplishing the above this requires splitting
string_wrapper into two separate classes, as we continue to need the
string_wrapper instance to handle the signature type, but filename() and
parent_path() on signature are non-sensical. Therefore, this splits the
functionality into string_wrapper and string_path_wrapper, each of which
no longer need to be a template, given there is only one use. We could
also get rid of the using, and move these classes out of details, but
that seemed better reserved for a later patch.
Tested:
Unit tests written and passing.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I242d1965875ba1fe76a32fd78e381e90796706fc
diff --git a/include/sdbusplus/message/append.hpp b/include/sdbusplus/message/append.hpp
index fbd99bf..b67e4a6 100644
--- a/include/sdbusplus/message/append.hpp
+++ b/include/sdbusplus/message/append.hpp
@@ -221,8 +221,20 @@
};
/** @brief Specialization of append_single for details::string_wrapper. */
-template <typename T>
-struct append_single<details::string_wrapper<T>>
+template <>
+struct append_single<details::string_wrapper>
+{
+ template <typename S>
+ static void op(sdbusplus::SdBusInterface* intf, sd_bus_message* m, S&& s)
+ {
+ constexpr auto dbusType = std::get<0>(types::type_id<S>());
+ intf->sd_bus_message_append_basic(m, dbusType, s.str.c_str());
+ }
+};
+
+/** @brief Specialization of append_single for details::string_wrapper. */
+template <>
+struct append_single<details::string_path_wrapper>
{
template <typename S>
static void op(sdbusplus::SdBusInterface* intf, sd_bus_message* m, S&& s)
diff --git a/include/sdbusplus/message/native_types.hpp b/include/sdbusplus/message/native_types.hpp
index 7bf5fb9..6506b70 100644
--- a/include/sdbusplus/message/native_types.hpp
+++ b/include/sdbusplus/message/native_types.hpp
@@ -13,7 +13,6 @@
/** Simple wrapper class for std::string to allow conversion to and from an
* alternative typename. */
-template <typename T>
struct string_wrapper
{
std::string str;
@@ -32,22 +31,22 @@
operator const std::string &() const volatile&
{
- return const_cast<const string_wrapper<T>*>(this)->str;
+ return const_cast<const string_wrapper*>(this)->str;
}
operator std::string &&() &&
{
return std::move(str);
}
- bool operator==(const string_wrapper<T>& r) const
+ bool operator==(const string_wrapper& r) const
{
return str == r.str;
}
- bool operator!=(const string_wrapper<T>& r) const
+ bool operator!=(const string_wrapper& r) const
{
return str != r.str;
}
- bool operator<(const string_wrapper<T>& r) const
+ bool operator<(const string_wrapper& r) const
{
return str < r.str;
}
@@ -77,10 +76,103 @@
return l < r.str;
}
};
+/** Simple wrapper class for std::string to allow conversion to and from an
+ * alternative typename. */
+struct string_path_wrapper
+{
+ std::string str;
-/** Typename for sdbus OBJECT_PATH types. */
-struct object_path_type
-{};
+ string_path_wrapper() = default;
+ string_path_wrapper(const string_path_wrapper&) = default;
+ string_path_wrapper& operator=(const string_path_wrapper&) = default;
+ string_path_wrapper(string_path_wrapper&&) = default;
+ string_path_wrapper& operator=(string_path_wrapper&&) = default;
+ ~string_path_wrapper() = default;
+
+ string_path_wrapper(const std::string& str) : str(str)
+ {}
+ string_path_wrapper(std::string&& str) : str(std::move(str))
+ {}
+
+ operator const std::string &() const volatile&
+ {
+ return const_cast<const string_path_wrapper*>(this)->str;
+ }
+ operator std::string &&() &&
+ {
+ return std::move(str);
+ }
+
+ bool operator==(const string_path_wrapper& r) const
+ {
+ return str == r.str;
+ }
+ bool operator!=(const string_path_wrapper& r) const
+ {
+ return str != r.str;
+ }
+ bool operator<(const string_path_wrapper& r) const
+ {
+ return str < r.str;
+ }
+ bool operator==(const std::string& r) const
+ {
+ return str == r;
+ }
+ bool operator!=(const std::string& r) const
+ {
+ return str != r;
+ }
+ bool operator<(const std::string& r) const
+ {
+ return str < r;
+ }
+
+ friend bool operator==(const std::string& l, const string_path_wrapper& r)
+ {
+ return l == r.str;
+ }
+ friend bool operator!=(const std::string& l, const string_path_wrapper& r)
+ {
+ return l != r.str;
+ }
+ friend bool operator<(const std::string& l, const string_path_wrapper& r)
+ {
+ return l < r.str;
+ }
+
+ std::string filename() const
+ {
+ auto index = str.rfind('/');
+ if (index == std::string::npos)
+ {
+ return "";
+ }
+ index++;
+ if (index >= str.size())
+ {
+ return "";
+ }
+
+ return str.substr(index);
+ }
+
+ string_path_wrapper parent_path() const
+ {
+ auto index = str.rfind('/');
+ if (index == std::string::npos)
+ {
+ return string_path_wrapper("/");
+ }
+ if (index <= 1)
+ {
+ return string_path_wrapper("/");
+ }
+
+ return str.substr(0, index);
+ }
+};
+
/** Typename for sdbus SIGNATURE types. */
struct signature_type
{};
@@ -102,9 +194,9 @@
} // namespace details
/** std::string wrapper for OBJECT_PATH. */
-using object_path = details::string_wrapper<details::object_path_type>;
+using object_path = details::string_path_wrapper;
/** std::string wrapper for SIGNATURE. */
-using signature = details::string_wrapper<details::signature_type>;
+using signature = details::string_wrapper;
using unix_fd = details::unix_fd_type;
} // namespace message
@@ -114,10 +206,23 @@
{
/** Overload of std::hash for details::string_wrappers */
-template <typename T>
-struct hash<sdbusplus::message::details::string_wrapper<T>>
+template <>
+struct hash<sdbusplus::message::details::string_wrapper>
{
- using argument_type = sdbusplus::message::details::string_wrapper<T>;
+ using argument_type = sdbusplus::message::details::string_wrapper;
+ using result_type = std::size_t;
+
+ result_type operator()(argument_type const& s) const
+ {
+ return hash<std::string>()(s.str);
+ }
+};
+
+/** Overload of std::hash for details::string_wrappers */
+template <>
+struct hash<sdbusplus::message::details::string_path_wrapper>
+{
+ using argument_type = sdbusplus::message::details::string_path_wrapper;
using result_type = std::size_t;
result_type operator()(argument_type const& s) const
diff --git a/include/sdbusplus/message/read.hpp b/include/sdbusplus/message/read.hpp
index 5024196..b30c867 100644
--- a/include/sdbusplus/message/read.hpp
+++ b/include/sdbusplus/message/read.hpp
@@ -208,8 +208,27 @@
};
/** @brief Specialization of read_single for details::string_wrapper. */
-template <typename T>
-struct read_single<details::string_wrapper<T>>
+template <>
+struct read_single<details::string_wrapper>
+{
+ template <typename S>
+ static void op(sdbusplus::SdBusInterface* intf, sd_bus_message* m, S&& s)
+ {
+ constexpr auto dbusType = std::get<0>(types::type_id<S>());
+ const char* str = nullptr;
+ int r = intf->sd_bus_message_read_basic(m, dbusType, &str);
+ if (r < 0)
+ {
+ throw exception::SdBusError(
+ -r, "sd_bus_message_read_basic string_wrapper");
+ }
+ s.str = str;
+ }
+};
+
+/** @brief Specialization of read_single for details::string_wrapper. */
+template <>
+struct read_single<details::string_path_wrapper>
{
template <typename S>
static void op(sdbusplus::SdBusInterface* intf, sd_bus_message* m, S&& s)
diff --git a/test/message/types.cpp b/test/message/types.cpp
index 89cab14..9f82f0a 100644
--- a/test/message/types.cpp
+++ b/test/message/types.cpp
@@ -41,6 +41,28 @@
ASSERT_EQ(dbus_string(sdbusplus::message::object_path("/asdf")), "o");
}
+TEST(MessageTypes, ObjectPathLeaf)
+{
+ ASSERT_EQ(sdbusplus::message::object_path("/abc/def").filename(), "def");
+ ASSERT_EQ(sdbusplus::message::object_path("/abc/").filename(), "");
+ ASSERT_EQ(sdbusplus::message::object_path("/abc").filename(), "abc");
+ ASSERT_EQ(sdbusplus::message::object_path("/").filename(), "");
+ ASSERT_EQ(sdbusplus::message::object_path("").filename(), "");
+ ASSERT_EQ(sdbusplus::message::object_path("abc").filename(), "");
+}
+
+TEST(MessageTypes, ObjectPathParent)
+{
+ ASSERT_EQ(sdbusplus::message::object_path("/abc/def").parent_path(),
+ sdbusplus::message::object_path("/abc"));
+ ASSERT_EQ(sdbusplus::message::object_path("/abc/").parent_path(),
+ sdbusplus::message::object_path("/abc"));
+ ASSERT_EQ(sdbusplus::message::object_path("/abc").parent_path(),
+ sdbusplus::message::object_path("/"));
+ ASSERT_EQ(sdbusplus::message::object_path("/").parent_path(),
+ sdbusplus::message::object_path("/"));
+}
+
TEST(MessageTypes, Signature)
{
ASSERT_EQ(dbus_string(sdbusplus::message::signature("sss")), "g");