From 0acfe8d562d7ec3aa5fc12c4f6add3821bbd2656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanislav=20Angelovi=C4=8D?= Date: Tue, 31 Oct 2023 13:41:08 +0100 Subject: [PATCH] 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(). --- src/Connection.cpp | 30 ++++++++---------------------- src/Connection.h | 4 ---- src/ISdBus.h | 1 + src/SdBus.cpp | 7 +++++++ src/SdBus.h | 1 + tests/unittests/mocks/SdBusMock.h | 1 + 6 files changed, 18 insertions(+), 26 deletions(-) diff --git a/src/Connection.cpp b/src/Connection.cpp index 74d88c3f..6c6a6e3d 100644 --- 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(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 Connection::to_strv(const std::vector& strings) { std::vector strv; diff --git a/src/Connection.h b/src/Connection.h index a73422e9..2512ad89 100644 --- 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 index d0169127..e5722741 100644 --- 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 index 72767908..713370e9 100644 --- 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 index fb7f087c..ab947b1a 100644 --- 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 index 65cc6f14..f33fcc91 100644 --- 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));