refactor: use sd_bus_match_signal() for signal registration (#372)

sd_bus_match_signal() is more convenient than more general sd_bus_add_match() for signal registration, and requires less code on our side. Plus, systemd of at least v238 is required for sdbus-c++ v2, and this version already ships with sd_bus_match_signal().
Kistler-Group · Dec 5, 2023 · 0acfe8d · 0acfe8d
1 parent 690f11b
commit 0acfe8d
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 26 deletions.
diff --git a/src/Connection.cpp b/src/Connection.cpp
@@ -626,11 +626,14 @@ Slot Connection::registerSignalHandler( const std::string& sender
 {
     sd_bus_slot *slot{};
 
-    // Alternatively to our own composeSignalMatchFilter() implementation, we could use sd_bus_match_signal() from
-    // https://www.freedesktop.org/software/systemd/man/sd_bus_add_match.html .
-    // But this would require libsystemd v237 or higher.
-    auto filter = composeSignalMatchFilter(sender, objectPath, interfaceName, signalName);
-    auto r = sdbus_->sd_bus_add_match(bus_.get(), &slot, filter.c_str(), callback, userData);
+    auto r = sdbus_->sd_bus_match_signal( bus_.get()
+                                        , &slot
+                                        , !sender.empty() ? sender.c_str() : nullptr
+                                        , !objectPath.empty() ? objectPath.c_str() : nullptr
+                                        , !interfaceName.empty() ? interfaceName.c_str() : nullptr
+                                        , !signalName.empty() ? signalName.c_str() : nullptr
+                                        , callback
+                                        , userData );
 
     SDBUS_THROW_ERROR_IF(r < 0, "Failed to register signal handler", -r);
 
@@ -767,23 +770,6 @@ Message Connection::getCurrentlyProcessedMessage() const
     return Message::Factory::create<Message>(sdbusMsg, sdbus_.get());
 }
 
-std::string Connection::composeSignalMatchFilter( const std::string &sender
-                                                , const std::string &objectPath
-                                                , const std::string &interfaceName
-                                                , const std::string &signalName )
-{
-    std::string filter;
-
-    filter += "type='signal',";
-    if (!sender.empty())
-        filter += "sender='" + sender + "',";
-    filter += "interface='" + interfaceName + "',";
-    filter += "member='" + signalName + "',";
-    filter += "path='" + objectPath + "'";
-
-    return filter;
-}
-
 std::vector</*const */char*> Connection::to_strv(const std::vector<std::string>& strings)
 {
     std::vector</*const */char*> strv;

diff --git a/src/Connection.h b/src/Connection.h
@@ -153,10 +153,6 @@ namespace sdbus::internal {
         bool waitForNextEvent();
 
         bool arePendingMessagesInReadQueue() const;
-        static std::string composeSignalMatchFilter( const std::string &sender
-                                                   , const std::string &objectPath
-                                                   , const std::string &interfaceName
-                                                   , const std::string &signalName);
 
         void notifyEventLoopToExit();
         void notifyEventLoopToWakeUpFromPoll();

diff --git a/src/ISdBus.h b/src/ISdBus.h
@@ -81,6 +81,7 @@ namespace sdbus::internal {
         virtual int sd_bus_add_object_manager(sd_bus *bus, sd_bus_slot **slot, const char *path) = 0;
         virtual int sd_bus_add_match(sd_bus *bus, sd_bus_slot **slot, const char *match, sd_bus_message_handler_t callback, void *userdata) = 0;
         virtual int sd_bus_add_match_async(sd_bus *bus, sd_bus_slot **slot, const char *match, sd_bus_message_handler_t callback, sd_bus_message_handler_t install_callback, void *userdata) = 0;
+        virtual int sd_bus_match_signal(sd_bus *bus, sd_bus_slot **ret, const char *sender, const char *path, const char *interface, const char *member, sd_bus_message_handler_t callback, void *userdata) = 0;
         virtual sd_bus_slot* sd_bus_slot_unref(sd_bus_slot *slot) = 0;
 
         virtual int sd_bus_new(sd_bus **ret) = 0;

diff --git a/src/SdBus.cpp b/src/SdBus.cpp
@@ -358,6 +358,13 @@ int SdBus::sd_bus_add_match_async(sd_bus *bus, sd_bus_slot **slot, const char *m
     return ::sd_bus_add_match_async(bus, slot, match, callback, install_callback, userdata);
 }
 
+int SdBus::sd_bus_match_signal(sd_bus *bus, sd_bus_slot **ret, const char *sender, const char *path, const char *interface, const char *member, sd_bus_message_handler_t callback, void *userdata)
+{
+    std::lock_guard lock(sdbusMutex_);
+
+    return ::sd_bus_match_signal(bus, ret, sender, path, interface, member, callback, userdata);
+}
+
 sd_bus_slot* SdBus::sd_bus_slot_unref(sd_bus_slot *slot)
 {
     std::lock_guard lock(sdbusMutex_);

diff --git a/src/SdBus.h b/src/SdBus.h
@@ -73,6 +73,7 @@ class SdBus final : public ISdBus
     virtual int sd_bus_add_object_manager(sd_bus *bus, sd_bus_slot **slot, const char *path) override;
     virtual int sd_bus_add_match(sd_bus *bus, sd_bus_slot **slot, const char *match, sd_bus_message_handler_t callback, void *userdata) override;
     virtual int sd_bus_add_match_async(sd_bus *bus, sd_bus_slot **slot, const char *match, sd_bus_message_handler_t callback, sd_bus_message_handler_t install_callback, void *userdata) override;
+    virtual int sd_bus_match_signal(sd_bus *bus, sd_bus_slot **ret, const char *sender, const char *path, const char *interface, const char *member, sd_bus_message_handler_t callback, void *userdata) override;
     virtual sd_bus_slot* sd_bus_slot_unref(sd_bus_slot *slot) override;
 
     virtual int sd_bus_new(sd_bus **ret) override;

diff --git a/tests/unittests/mocks/SdBusMock.h b/tests/unittests/mocks/SdBusMock.h
@@ -72,6 +72,7 @@ class SdBusMock : public sdbus::internal::ISdBus
     MOCK_METHOD3(sd_bus_add_object_manager, int(sd_bus *bus, sd_bus_slot **slot, const char *path));
     MOCK_METHOD5(sd_bus_add_match, int(sd_bus *bus, sd_bus_slot **slot, const char *match, sd_bus_message_handler_t callback, void *userdata));
     MOCK_METHOD6(sd_bus_add_match_async, int(sd_bus *bus, sd_bus_slot **slot, const char *match, sd_bus_message_handler_t callback, sd_bus_message_handler_t install_callback, void *userdata));
+    MOCK_METHOD8(sd_bus_match_signal, int(sd_bus *bus, sd_bus_slot **ret, const char *sender, const char *path, const char *interface, const char *member, sd_bus_message_handler_t callback, void *userdata));
     MOCK_METHOD1(sd_bus_slot_unref, sd_bus_slot*(sd_bus_slot *slot));
 
     MOCK_METHOD1(sd_bus_new, int(sd_bus **ret));