Little code cleanups and refactorings

src/Connection.cpp

@@ -32,93 +32,48 @@
 #include <poll.h>
 #include <sys/eventfd.h>
 
-namespace {
-    std::map<sdbus::internal::Connection::BusType, int(*)(sd_bus **)> busTypeToFactory
-    {
-        {sdbus::internal::Connection::BusType::eSystem, &sd_bus_open_system},
-        {sdbus::internal::Connection::BusType::eSession, &sd_bus_open_user}
-    };
-}
-
 namespace sdbus { namespace internal {
 
 Connection::Connection(Connection::BusType type)
     : busType_(type)
 {
-    sd_bus* bus{};
+    auto bus = openBus(busType_);
-    auto r = busTypeToFactory[busType_](&bus);
-    if (r < 0)
-        SDBUS_THROW_ERROR("Failed to open system bus", -r);
-
     bus_.reset(bus);
 
-    // Process all requests that are part of the initial handshake,
+    finishHandshake(bus);
-    // like processing the Hello message response, authentication etc.,
-    // to avoid connection authentication timeout in dbus daemon.
-    r = sd_bus_flush(bus_.get());
-    if (r < 0)
-        SDBUS_THROW_ERROR("Failed to flush system bus on opening", -r);
 
-    r = eventfd(0, EFD_SEMAPHORE | EFD_CLOEXEC);
+    exitLoopFd_ = createProcessingExitDescriptor();
-    SDBUS_THROW_ERROR_IF(r < 0, "Failed to create event object", -errno);
-    runFd_ = r;
 }
 
 Connection::~Connection()
 {
     leaveProcessingLoop();
-    close(runFd_);
+    closeProcessingExitDescriptor(exitLoopFd_);
 }
 
 void Connection::requestName(const std::string& name)
 {
     auto r = sd_bus_request_name(bus_.get(), name.c_str(), 0);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to request bus name", -r);
-        SDBUS_THROW_ERROR("Failed to request bus name", -r);
 }
 
 void Connection::releaseName(const std::string& name)
 {
     auto r = sd_bus_release_name(bus_.get(), name.c_str());
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to release bus name", -r);
-        SDBUS_THROW_ERROR("Failed to release bus name", -r);
 }
 
 void Connection::enterProcessingLoop()
 {
-    int semaphoreFd = runFd_;
-    short int semaphoreEvents = POLLIN;
-
     while (true)
     {
-        /* Process requests */
+        auto processed = processPendingRequest(bus_.get());
-        int r = sd_bus_process(bus_.get(), nullptr);
+        if (processed)
-        SDBUS_THROW_ERROR_IF(r < 0, "Failed to process bus requests", -r);
+            continue; // Process next one
-        if (r > 0) /* we processed a request, try to process another one, right-away */
-            continue;
 
-        r = sd_bus_get_fd(bus_.get());
+        auto success = waitForNextRequest(bus_.get(), exitLoopFd_);
-        SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus descriptor", -r);
+        if (!success)
-        auto sdbusFd = r;
+            break; // Exit processing loop
-
-        r = sd_bus_get_events(bus_.get());
-        SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus descriptor", -r);
-        short int sdbusEvents = r;
-
-        struct pollfd fds[] = {{sdbusFd, sdbusEvents, 0}, {semaphoreFd, semaphoreEvents, 0}};
-
-        /* Wait for the next request to process */
-        uint64_t usec;
-        sd_bus_get_timeout(bus_.get(), &usec);
-
-        auto fdsCount = sizeof(fds)/sizeof(fds[0]);
-        r = poll(fds, fdsCount, usec == (uint64_t) -1 ? -1 : (usec+999)/1000);
-        if (r < 0 && errno == EINTR)
-            continue;
-        SDBUS_THROW_ERROR_IF(r < 0, "Failed to wait on the bus", -errno);
-
-        if (fds[1].revents & POLLIN)
-            break;
     }
 }
 
@@ -129,12 +84,8 @@ void Connection::enterProcessingLoopAsync()
 
 void Connection::leaveProcessingLoop()
 {
-    assert(runFd_ >= 0);
+    notifyProcessingLoopToExit();
-    uint64_t value = 1;
+    joinWithProcessingLoop();
-    write(runFd_, &value, sizeof(value));
-
-    if (asyncLoopThread_.joinable())
-        asyncLoopThread_.join();
 }
 
 void* Connection::addObjectVTable( const std::string& objectPath
@@ -143,14 +94,15 @@ void* Connection::addObjectVTable( const std::string& objectPath
                                  , void* userData )
 {
     sd_bus_slot *slot{};
+
     auto r = sd_bus_add_object_vtable( bus_.get()
                                      , &slot
                                      , objectPath.c_str()
                                      , interfaceName.c_str()
                                      , static_cast<const sd_bus_vtable*>(vtable)
                                      , userData );
-    if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to register object vtable", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to register object vtable", -r);
 
     return slot;
 }
@@ -166,15 +118,18 @@ sdbus::Message Connection::createMethodCall( const std::string& destination
                                            , const std::string& methodName ) const
 {
     sd_bus_message *sdbusMsg{};
-    SCOPE_EXIT{ sd_bus_message_unref(sdbusMsg); }; // Returned message will become an owner of sdbusMsg
+
+    // Returned message will become an owner of sdbusMsg
+    SCOPE_EXIT{ sd_bus_message_unref(sdbusMsg); };
+
     auto r = sd_bus_message_new_method_call( bus_.get()
                                            , &sdbusMsg
                                            , destination.c_str()
                                            , objectPath.c_str()
                                            , interfaceName.c_str()
                                            , methodName.c_str() );
-    if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to create method call", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to create method call", -r);
 
     return Message(sdbusMsg, Message::Type::eMethodCall);
 }
@@ -184,14 +139,17 @@ sdbus::Message Connection::createSignal( const std::string& objectPath
                                        , const std::string& signalName ) const
 {
     sd_bus_message *sdbusSignal{};
-    SCOPE_EXIT{ sd_bus_message_unref(sdbusSignal); }; // Returned message will become an owner of sdbusSignal
+
+    // Returned message will become an owner of sdbusSignal
+    SCOPE_EXIT{ sd_bus_message_unref(sdbusSignal); };
+
     auto r = sd_bus_message_new_signal( bus_.get()
                                       , &sdbusSignal
                                       , objectPath.c_str()
                                       , interfaceName.c_str()
                                       , signalName.c_str() );
-    if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to create signal", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to create signal", -r);
 
     return Message(sdbusSignal, Message::Type::eSignal);
 }
@@ -203,10 +161,11 @@ void* Connection::registerSignalHandler( const std::string& objectPath
                                        , void* userData )
 {
     sd_bus_slot *slot{};
+
     auto filter = composeSignalMatchFilter(objectPath, interfaceName, signalName);
     auto r = sd_bus_add_match(bus_.get(), &slot, filter.c_str(), callback, userData);
-    if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to register signal handler", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to register signal handler", -r);
 
     return slot;
 }
@@ -221,15 +180,120 @@ std::unique_ptr<sdbus::internal::IConnection> Connection::clone() const
     return std::make_unique<sdbus::internal::Connection>(busType_);
 }
 
+sd_bus* Connection::openBus(Connection::BusType type)
+{
+    static std::map<sdbus::internal::Connection::BusType, int(*)(sd_bus **)> busTypeToFactory
+    {
+        {sdbus::internal::Connection::BusType::eSystem, &sd_bus_open_system},
+        {sdbus::internal::Connection::BusType::eSession, &sd_bus_open_user}
+    };
+
+    sd_bus* bus{};
+
+    auto r = busTypeToFactory[type](&bus);
+
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to open bus", -r);
+    assert(bus != nullptr);
+
+    return bus;
+}
+
+void Connection::finishHandshake(sd_bus* bus)
+{
+    // Process all requests that are part of the initial handshake,
+    // like processing the Hello message response, authentication etc.,
+    // to avoid connection authentication timeout in dbus daemon.
+
+    assert(bus != nullptr);
+
+    auto r = sd_bus_flush(bus);
+
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to flush bus on opening", -r);
+}
+
+int Connection::createProcessingExitDescriptor()
+{
+    // Mechanism for graceful termination of processing loop
+
+    auto r = eventfd(0, EFD_SEMAPHORE | EFD_CLOEXEC);
+
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to create event object", -errno);
+
+    return r;
+}
+
+void Connection::closeProcessingExitDescriptor(int fd)
+{
+    close(fd);
+}
+
+void Connection::notifyProcessingLoopToExit()
+{
+    assert(exitLoopFd_ >= 0);
+    uint64_t value = 1;
+    write(exitLoopFd_, &value, sizeof(value));
+}
+
+void Connection::joinWithProcessingLoop()
+{
+    if (asyncLoopThread_.joinable())
+        asyncLoopThread_.join();
+}
+
+bool Connection::processPendingRequest(sd_bus* bus)
+{
+    assert(bus != nullptr);
+
+    int r = sd_bus_process(bus, nullptr);
+
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to process bus requests", -r);
+
+    return r > 0;
+}
+
+bool Connection::waitForNextRequest(sd_bus* bus, int exitFd)
+{
+    assert(bus != nullptr);
+    assert(exitFd != 0);
+
+    auto r = sd_bus_get_fd(bus);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus descriptor", -r);
+    auto sdbusFd = r;
+
+    r = sd_bus_get_events(bus);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus events", -r);
+    short int sdbusEvents = r;
+
+    uint64_t usec;
+    sd_bus_get_timeout(bus, &usec);
+
+    struct pollfd fds[] = {{sdbusFd, sdbusEvents, 0}, {exitFd, POLLIN, 0}};
+    auto fdsCount = sizeof(fds)/sizeof(fds[0]);
+
+    r = poll(fds, fdsCount, usec == (uint64_t) -1 ? -1 : (usec+999)/1000);
+
+    if (r < 0 && errno == EINTR)
+        return true; // Try again
+
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to wait on the bus", -errno);
+
+    if (fds[1].revents & POLLIN)
+        return false; // Got exit notification
+
+    return true;
+}
+
 std::string Connection::composeSignalMatchFilter( const std::string& objectPath
                                                 , const std::string& interfaceName
                                                 , const std::string& signalName )
 {
     std::string filter;
+
     filter += "type='signal',";
     filter += "interface='" + interfaceName + "',";
     filter += "member='" + signalName + "',";
     filter += "path='" + objectPath + "'";
+
     return filter;
 }
 

src/Connection.h

@@ -80,14 +80,22 @@ namespace sdbus { namespace internal {
         std::unique_ptr<sdbus::internal::IConnection> clone() const override;
 
     private:
+        static sd_bus* openBus(Connection::BusType type);
+        static void finishHandshake(sd_bus* bus);
+        static int createProcessingExitDescriptor();
+        static void closeProcessingExitDescriptor(int fd);
+        static bool processPendingRequest(sd_bus* bus);
+        static bool waitForNextRequest(sd_bus* bus, int exitFd);
         static std::string composeSignalMatchFilter( const std::string& objectPath
                                                    , const std::string& interfaceName
                                                    , const std::string& signalName );
+        void notifyProcessingLoopToExit();
+        void joinWithProcessingLoop();
 
     private:
         std::unique_ptr<sd_bus, decltype(&sd_bus_flush_close_unref)> bus_{nullptr, &sd_bus_flush_close_unref};
         std::thread asyncLoopThread_;
-        std::atomic<int> runFd_{-1};
+        std::atomic<int> exitLoopFd_{-1};
         BusType busType_;
 
         static constexpr const uint64_t POLL_TIMEOUT_USEC = 500000;

src/Error.cpp

@@ -25,6 +25,7 @@
 
 #include <sdbus-c++/Error.h>
 #include <systemd/sd-bus.h>
+#include "ScopeGuard.h"
 
 namespace sdbus
 {
@@ -32,9 +33,10 @@ namespace sdbus
     {
         sd_bus_error sdbusError = SD_BUS_ERROR_NULL;
         sd_bus_error_set_errno(&sdbusError, errNo);
+        SCOPE_EXIT{ sd_bus_error_free(&sdbusError); };
+
         std::string name(sdbusError.name);
         std::string message(customMsg + " (" + sdbusError.message + ")");
-        sd_bus_error_free(&sdbusError);
         return sdbus::Error(name, message);
     }
 }

src/Message.cpp

@@ -85,8 +85,7 @@ Message& Message::operator<<(bool item)
     int intItem = item;
 
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_BOOLEAN, &intItem);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a bool value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize a bool value", -r);
 
     return *this;
 }
@@ -94,8 +93,7 @@ Message& Message::operator<<(bool item)
 Message& Message::operator<<(int16_t item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT16, &item);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a int16_t value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize a int16_t value", -r);
 
     return *this;
 }
@@ -103,8 +101,7 @@ Message& Message::operator<<(int16_t item)
 Message& Message::operator<<(int32_t item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT32, &item);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a int32_t value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize a int32_t value", -r);
 
     return *this;
 }
@@ -112,8 +109,7 @@ Message& Message::operator<<(int32_t item)
 Message& Message::operator<<(int64_t item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT64, &item);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a int64_t value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize a int64_t value", -r);
 
     return *this;
 }
@@ -121,8 +117,7 @@ Message& Message::operator<<(int64_t item)
 Message& Message::operator<<(uint8_t item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_BYTE, &item);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a byte value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize a byte value", -r);
 
     return *this;
 }
@@ -130,8 +125,7 @@ Message& Message::operator<<(uint8_t item)
 Message& Message::operator<<(uint16_t item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT16, &item);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a uint16_t value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize a uint16_t value", -r);
 
     return *this;
 }
@@ -139,8 +133,7 @@ Message& Message::operator<<(uint16_t item)
 Message& Message::operator<<(uint32_t item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT32, &item);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a uint32_t value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize a uint32_t value", -r);
 
     return *this;
 }
@@ -148,8 +141,7 @@ Message& Message::operator<<(uint32_t item)
 Message& Message::operator<<(uint64_t item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT64, &item);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a uint64_t value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize a uint64_t value", -r);
 
     return *this;
 }
@@ -157,8 +149,7 @@ Message& Message::operator<<(uint64_t item)
 Message& Message::operator<<(double item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_DOUBLE, &item);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a double value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize a double value", -r);
 
     return *this;
 }
@@ -166,8 +157,7 @@ Message& Message::operator<<(double item)
 Message& Message::operator<<(const char* item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_STRING, item);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a C-string value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize a C-string value", -r);
 
     return *this;
 }
@@ -175,8 +165,7 @@ Message& Message::operator<<(const char* item)
 Message& Message::operator<<(const std::string& item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_STRING, item.c_str());
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a string value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize a string value", -r);
 
     return *this;
 }
@@ -191,8 +180,7 @@ Message& Message::operator<<(const Variant &item)
 Message& Message::operator<<(const ObjectPath &item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_OBJECT_PATH, item.c_str());
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize an ObjectPath value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize an ObjectPath value", -r);
 
     return *this;
 }
@@ -200,8 +188,7 @@ Message& Message::operator<<(const ObjectPath &item)
 Message& Message::operator<<(const Signature &item)
 {
     auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_SIGNATURE, item.c_str());
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize an Signature value", -r);
-        SDBUS_THROW_ERROR("Failed to serialize an Signature value", -r);
 
     return *this;
 }
@@ -213,8 +200,8 @@ Message& Message::operator>>(bool& item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_BOOLEAN, &intItem);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize a bool value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a bool value", -r);
 
     item = static_cast<bool>(intItem);
 
@@ -226,8 +213,8 @@ Message& Message::operator>>(int16_t& item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT16, &item);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize a int16_t value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a int16_t value", -r);
 
     return *this;
 }
@@ -237,8 +224,8 @@ Message& Message::operator>>(int32_t& item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT32, &item);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize a int32_t value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a int32_t value", -r);
 
     return *this;
 }
@@ -248,8 +235,8 @@ Message& Message::operator>>(int64_t& item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT64, &item);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize a bool value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a bool value", -r);
 
     return *this;
 }
@@ -259,8 +246,8 @@ Message& Message::operator>>(uint8_t& item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_BYTE, &item);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize a byte value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a byte value", -r);
 
     return *this;
 }
@@ -270,8 +257,8 @@ Message& Message::operator>>(uint16_t& item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT16, &item);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize a uint16_t value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a uint16_t value", -r);
 
     return *this;
 }
@@ -281,8 +268,8 @@ Message& Message::operator>>(uint32_t& item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT32, &item);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize a uint32_t value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a uint32_t value", -r);
 
     return *this;
 }
@@ -292,8 +279,8 @@ Message& Message::operator>>(uint64_t& item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT64, &item);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize a uint64_t value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a uint64_t value", -r);
 
     return *this;
 }
@@ -303,8 +290,8 @@ Message& Message::operator>>(double& item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_DOUBLE, &item);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize a double value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a double value", -r);
 
     return *this;
 }
@@ -314,8 +301,8 @@ Message& Message::operator>>(char*& item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_STRING, &item);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize a string value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a string value", -r);
 
     return *this;
 }
@@ -350,8 +337,8 @@ Message& Message::operator>>(ObjectPath &item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_OBJECT_PATH, &str);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize an ObjectPath value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize an ObjectPath value", -r);
 
     if (str != nullptr)
         item = str;
@@ -365,8 +352,8 @@ Message& Message::operator>>(Signature &item)
     auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_SIGNATURE, &str);
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to deserialize a Signature value", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a Signature value", -r);
 
     if (str != nullptr)
         item = str;
@@ -378,8 +365,7 @@ Message& Message::operator>>(Signature &item)
 Message& Message::openContainer(const std::string& signature)
 {
     auto r = sd_bus_message_open_container((sd_bus_message*)msg_, SD_BUS_TYPE_ARRAY, signature.c_str());
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to open a container", -r);
-        SDBUS_THROW_ERROR("Failed to open a container", -r);
 
     return *this;
 }
@@ -387,8 +373,7 @@ Message& Message::openContainer(const std::string& signature)
 Message& Message::closeContainer()
 {
     auto r = sd_bus_message_close_container((sd_bus_message*)msg_);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to close a container", -r);
-        SDBUS_THROW_ERROR("Failed to close a container", -r);
 
     return *this;
 }
@@ -396,8 +381,7 @@ Message& Message::closeContainer()
 Message& Message::openDictEntry(const std::string& signature)
 {
     auto r = sd_bus_message_open_container((sd_bus_message*)msg_, SD_BUS_TYPE_DICT_ENTRY, signature.c_str());
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to open a dictionary entry", -r);
-        SDBUS_THROW_ERROR("Failed to open a dictionary entry", -r);
 
     return *this;
 }
@@ -405,8 +389,7 @@ Message& Message::openDictEntry(const std::string& signature)
 Message& Message::closeDictEntry()
 {
     auto r = sd_bus_message_close_container((sd_bus_message*)msg_);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to close a dictionary entry", -r);
-        SDBUS_THROW_ERROR("Failed to close a dictionary entry", -r);
 
     return *this;
 }
@@ -414,8 +397,7 @@ Message& Message::closeDictEntry()
 Message& Message::openVariant(const std::string& signature)
 {
     auto r = sd_bus_message_open_container((sd_bus_message*)msg_, SD_BUS_TYPE_VARIANT, signature.c_str());
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to open a variant", -r);
-        SDBUS_THROW_ERROR("Failed to open a variant", -r);
 
     return *this;
 }
@@ -423,8 +405,7 @@ Message& Message::openVariant(const std::string& signature)
 Message& Message::closeVariant()
 {
     auto r = sd_bus_message_close_container((sd_bus_message*)msg_);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to close a variant", -r);
-        SDBUS_THROW_ERROR("Failed to close a variant", -r);
 
     return *this;
 }
@@ -432,8 +413,7 @@ Message& Message::closeVariant()
 Message& Message::openStruct(const std::string& signature)
 {
     auto r = sd_bus_message_open_container((sd_bus_message*)msg_, SD_BUS_TYPE_STRUCT, signature.c_str());
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to open a struct", -r);
-        SDBUS_THROW_ERROR("Failed to open a struct", -r);
 
     return *this;
 }
@@ -441,8 +421,7 @@ Message& Message::openStruct(const std::string& signature)
 Message& Message::closeStruct()
 {
     auto r = sd_bus_message_close_container((sd_bus_message*)msg_);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to close a struct", -r);
-        SDBUS_THROW_ERROR("Failed to close a struct", -r);
 
     return *this;
 }
@@ -453,8 +432,8 @@ Message& Message::enterContainer(const std::string& signature)
     auto r = sd_bus_message_enter_container((sd_bus_message*)msg_, SD_BUS_TYPE_ARRAY, signature.c_str());
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to enter a container", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to enter a container", -r);
 
     return *this;
 }
@@ -462,8 +441,7 @@ Message& Message::enterContainer(const std::string& signature)
 Message& Message::exitContainer()
 {
     auto r = sd_bus_message_exit_container((sd_bus_message*)msg_);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to exit a container", -r);
-        SDBUS_THROW_ERROR("Failed to exit a container", -r);
 
     return *this;
 }
@@ -473,8 +451,8 @@ Message& Message::enterDictEntry(const std::string& signature)
     auto r = sd_bus_message_enter_container((sd_bus_message*)msg_, SD_BUS_TYPE_DICT_ENTRY, signature.c_str());
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to enter a dictionary entry", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to enter a dictionary entry", -r);
 
     return *this;
 }
@@ -482,8 +460,7 @@ Message& Message::enterDictEntry(const std::string& signature)
 Message& Message::exitDictEntry()
 {
     auto r = sd_bus_message_exit_container((sd_bus_message*)msg_);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to exit a dictionary entry", -r);
-        SDBUS_THROW_ERROR("Failed to exit a dictionary entry", -r);
 
     return *this;
 }
@@ -493,8 +470,8 @@ Message& Message::enterVariant(const std::string& signature)
     auto r = sd_bus_message_enter_container((sd_bus_message*)msg_, SD_BUS_TYPE_VARIANT, signature.c_str());
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to enter a variant", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to enter a variant", -r);
 
     return *this;
 }
@@ -502,8 +479,7 @@ Message& Message::enterVariant(const std::string& signature)
 Message& Message::exitVariant()
 {
     auto r = sd_bus_message_exit_container((sd_bus_message*)msg_);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to exit a variant", -r);
-        SDBUS_THROW_ERROR("Failed to exit a variant", -r);
 
     return *this;
 }
@@ -513,8 +489,8 @@ Message& Message::enterStruct(const std::string& signature)
     auto r = sd_bus_message_enter_container((sd_bus_message*)msg_, SD_BUS_TYPE_STRUCT, signature.c_str());
     if (r == 0)
         ok_ = false;
-    else if (r < 0)
+
-        SDBUS_THROW_ERROR("Failed to enter a struct", -r);
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to enter a struct", -r);
 
     return *this;
 }
@@ -522,8 +498,7 @@ Message& Message::enterStruct(const std::string& signature)
 Message& Message::exitStruct()
 {
     auto r = sd_bus_message_exit_container((sd_bus_message*)msg_);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to exit a struct", -r);
-        SDBUS_THROW_ERROR("Failed to exit a struct", -r);
 
     return *this;
 }
@@ -542,8 +517,7 @@ void Message::clearFlags()
 void Message::copyTo(Message& destination, bool complete) const
 {
     auto r = sd_bus_message_copy((sd_bus_message*)destination.msg_, (sd_bus_message*)msg_, complete);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to copy the message", -r);
-        SDBUS_THROW_ERROR("Failed to copy the message", -r);
 }
 
 void Message::seal()
@@ -551,15 +525,13 @@ void Message::seal()
     const auto messageCookie = 1;
     const auto sealTimeout = 0;
     auto r = sd_bus_message_seal((sd_bus_message*)msg_, messageCookie, sealTimeout);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to seal the message", -r);
-        SDBUS_THROW_ERROR("Failed to seal the message", -r);
 }
 
 void Message::rewind(bool complete)
 {
     auto r = sd_bus_message_rewind((sd_bus_message*)msg_, complete);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to rewind the message", -r);
-        SDBUS_THROW_ERROR("Failed to rewind the message", -r);
 }
 
 Message Message::send() const
@@ -585,16 +557,14 @@ Message Message::send() const
     else if (type_ == Type::eMethodReply)
     {
         auto r = sd_bus_send(nullptr, (sd_bus_message*)msg_, nullptr);
-        if (r < 0)
+        SDBUS_THROW_ERROR_IF(r < 0, "Failed to send reply", -r);
-            SDBUS_THROW_ERROR("Failed to send reply", -r);
 
         return Message();
     }
     else if (type_ == Type::eSignal)
     {
         auto r = sd_bus_send(nullptr, (sd_bus_message*)msg_, nullptr);
-        if (r < 0)
+        SDBUS_THROW_ERROR_IF(r < 0, "Failed to emit signal", -r);
-            SDBUS_THROW_ERROR("Failed to emit signal", -r);
 
         return Message();
     }
@@ -610,8 +580,7 @@ Message Message::createReply() const
     sd_bus_message *sdbusReply{};
     SCOPE_EXIT{ sd_bus_message_unref(sdbusReply); }; // Returned message will become an owner of sdbusReply
     auto r = sd_bus_message_new_method_return((sd_bus_message*)msg_, &sdbusReply);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to create method reply", -r);
-        SDBUS_THROW_ERROR("Failed to create method reply", -r);
 
     assert(sdbusReply != nullptr);
 
@@ -633,8 +602,7 @@ void Message::peekType(std::string& type, std::string& contents) const
     char typeSig;
     const char* contentsSig;
     auto r = sd_bus_message_peek_type((sd_bus_message*)msg_, &typeSig, &contentsSig);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to peek message type", -r);
-        SDBUS_THROW_ERROR("Failed to peek message type", -r);
     type = typeSig;
     contents = contentsSig;
 }
@@ -661,14 +629,12 @@ Message createPlainMessage()
     sd_bus* bus{};
     SCOPE_EXIT{ sd_bus_unref(bus); }; // sdbusMsg will hold reference to the bus
     r = sd_bus_default_system(&bus);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to get default system bus", -r);
-        SDBUS_THROW_ERROR("Failed to get default system bus", -r);
 
     sd_bus_message* sdbusMsg{};
     SCOPE_EXIT{ sd_bus_message_unref(sdbusMsg); }; // Returned message will become an owner of sdbusMsg
     r = sd_bus_message_new(bus, &sdbusMsg, _SD_BUS_MESSAGE_TYPE_INVALID);
-    if (r < 0)
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to create a new message", -r);
-        SDBUS_THROW_ERROR("Failed to create a new message", -r);
 
     return Message(sdbusMsg, Message::Type::ePlainMessage);
 }

src/Object.cpp

@@ -103,49 +103,8 @@ void Object::finishRegistration()
         const auto& interfaceName = item.first;
         auto& interfaceData = item.second;
 
-        auto& vtable = interfaceData.vtable_;
+        const auto& vtable = createInterfaceVTable(interfaceData);
-        assert(vtable.empty());
+        activateInterfaceVTable(interfaceName, interfaceData, vtable);
-
-        vtable.push_back(createVTableStartItem());
-        for (const auto& item : interfaceData.methods_)
-        {
-            const auto& methodName = item.first;
-            const auto& methodData = item.second;
-
-            vtable.push_back(createVTableMethodItem( methodName.c_str()
-                                                   , methodData.inputArgs_.c_str()
-                                                   , methodData.outputArgs_.c_str()
-                                                   , &Object::sdbus_method_callback ));
-        }
-        for (const auto& item : interfaceData.signals_)
-        {
-            const auto& signalName = item.first;
-            const auto& signalData = item.second;
-
-            vtable.push_back(createVTableSignalItem( signalName.c_str()
-                                                   , signalData.signature_.c_str() ));
-        }
-        for (const auto& item : interfaceData.properties_)
-        {
-            const auto& propertyName = item.first;
-            const auto& propertyData = item.second;
-
-            if (!propertyData.setCallback_)
-                vtable.push_back(createVTablePropertyItem( propertyName.c_str()
-                                                         , propertyData.signature_.c_str()
-                                                         , &Object::sdbus_property_get_callback ));
-            else
-                vtable.push_back(createVTableWritablePropertyItem( propertyName.c_str()
-                                                                 , propertyData.signature_.c_str()
-                                                                 , &Object::sdbus_property_get_callback
-                                                                 , &Object::sdbus_property_set_callback ));
-        }
-        vtable.push_back(createVTableEndItem());
-
-        // Tell, don't ask
-        auto slot = (sd_bus_slot*) connection_.addObjectVTable(objectPath_, interfaceName, &vtable[0], this);
-        interfaceData.slot_.reset(slot);
-        interfaceData.slot_.get_deleter() = [this](void *slot){ connection_.removeObjectVTable(slot); };
     }
 }
 
@@ -160,6 +119,75 @@ void Object::emitSignal(const sdbus::Message& message)
     message.send();
 }
 
+const std::vector<sd_bus_vtable>& Object::createInterfaceVTable(InterfaceData& interfaceData)
+{
+    auto& vtable = interfaceData.vtable_;
+    assert(vtable.empty());
+
+    vtable.push_back(createVTableStartItem());
+    registerMethodsToVTable(interfaceData, vtable);
+    registerSignalsToVTable(interfaceData, vtable);
+    registerPropertiesToVTable(interfaceData, vtable);
+    vtable.push_back(createVTableEndItem());
+
+    return vtable;
+}
+
+void Object::registerMethodsToVTable(const InterfaceData& interfaceData, std::vector<sd_bus_vtable>& vtable)
+{
+    for (const auto& item : interfaceData.methods_)
+    {
+        const auto& methodName = item.first;
+        const auto& methodData = item.second;
+
+        vtable.push_back(createVTableMethodItem( methodName.c_str()
+                                               , methodData.inputArgs_.c_str()
+                                               , methodData.outputArgs_.c_str()
+                                               , &Object::sdbus_method_callback ));
+    }
+}
+
+void Object::registerSignalsToVTable(const InterfaceData& interfaceData, std::vector<sd_bus_vtable>& vtable)
+{
+    for (const auto& item : interfaceData.signals_)
+    {
+        const auto& signalName = item.first;
+        const auto& signalData = item.second;
+
+        vtable.push_back(createVTableSignalItem( signalName.c_str()
+                                               , signalData.signature_.c_str() ));
+    }
+}
+
+void Object::registerPropertiesToVTable(const InterfaceData& interfaceData, std::vector<sd_bus_vtable>& vtable)
+{
+    for (const auto& item : interfaceData.properties_)
+    {
+        const auto& propertyName = item.first;
+        const auto& propertyData = item.second;
+
+        if (!propertyData.setCallback_)
+            vtable.push_back(createVTablePropertyItem( propertyName.c_str()
+                                                     , propertyData.signature_.c_str()
+                                                     , &Object::sdbus_property_get_callback ));
+        else
+            vtable.push_back(createVTableWritablePropertyItem( propertyName.c_str()
+                                                             , propertyData.signature_.c_str()
+                                                             , &Object::sdbus_property_get_callback
+                                                             , &Object::sdbus_property_set_callback ));
+    }
+}
+
+void Object::activateInterfaceVTable( const std::string& interfaceName
+                                    , InterfaceData& interfaceData
+                                    , const std::vector<sd_bus_vtable>& vtable )
+{
+    // Tell, don't ask
+    auto slot = (sd_bus_slot*) connection_.addObjectVTable(objectPath_, interfaceName, &vtable[0], this);
+    interfaceData.slot_.reset(slot);
+    interfaceData.slot_.get_deleter() = [this](void *slot){ connection_.removeObjectVTable(slot); };
+}
+
 int Object::sdbus_method_callback(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError)
 {
     Message message(sdbusMessage, Message::Type::eMethodCall);

src/Object.h

@@ -71,26 +71,6 @@ namespace internal {
         void emitSignal(const sdbus::Message& message) override;
 
     private:
-        static int sdbus_method_callback(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError);
-        static int sdbus_property_get_callback( sd_bus *bus
-                                              , const char *objectPath
-                                              , const char *interface
-                                              , const char *property
-                                              , sd_bus_message *sdbusReply
-                                              , void *userData
-                                              , sd_bus_error *retError );
-        static int sdbus_property_set_callback( sd_bus *bus
-                                              , const char *objectPath
-                                              , const char *interface
-                                              , const char *property
-                                              , sd_bus_message *sdbusValue
-                                              , void *userData
-                                              , sd_bus_error *retError );
-
-    private:
-        sdbus::internal::IConnection& connection_;
-        std::string objectPath_;
-
         using InterfaceName = std::string;
         struct InterfaceData
         {
@@ -120,6 +100,34 @@ namespace internal {
 
             std::unique_ptr<void, std::function<void(void*)>> slot_;
         };
+
+        static const std::vector<sd_bus_vtable>& createInterfaceVTable(InterfaceData& interfaceData);
+        static void registerMethodsToVTable(const InterfaceData& interfaceData, std::vector<sd_bus_vtable>& vtable);
+        static void registerSignalsToVTable(const InterfaceData& interfaceData, std::vector<sd_bus_vtable>& vtable);
+        static void registerPropertiesToVTable(const InterfaceData& interfaceData, std::vector<sd_bus_vtable>& vtable);
+        void activateInterfaceVTable( const std::string& interfaceName
+                                    , InterfaceData& interfaceData
+                                    , const std::vector<sd_bus_vtable>& vtable );
+
+        static int sdbus_method_callback(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError);
+        static int sdbus_property_get_callback( sd_bus *bus
+                                              , const char *objectPath
+                                              , const char *interface
+                                              , const char *property
+                                              , sd_bus_message *sdbusReply
+                                              , void *userData
+                                              , sd_bus_error *retError );
+        static int sdbus_property_set_callback( sd_bus *bus
+                                              , const char *objectPath
+                                              , const char *interface
+                                              , const char *property
+                                              , sd_bus_message *sdbusValue
+                                              , void *userData
+                                              , sd_bus_error *retError );
+
+    private:
+        sdbus::internal::IConnection& connection_;
+        std::string objectPath_;
         std::map<InterfaceName, InterfaceData> interfaces_;
     };