refactor: little fixes and reorganizations around Connection class

include/sdbus-c++/IConnection.h

@@ -166,6 +166,12 @@ namespace sdbus {
          * the connection. This is a convenient way to interrogate a connection
          * to see what objects it has.
          *
+         * This call creates a floating registration. The ObjectManager will
+         * be there for the object path until the connection is destroyed.
+         *
+         * Another, recommended way to add object managers is directly through
+         * IObject API.
+         *
          * @throws sdbus::Error in case of failure
          */
         virtual void addObjectManager(const std::string& objectPath) = 0;

include/sdbus-c++/Message.h

@@ -32,7 +32,6 @@
 #include <string>
 #include <vector>
 #include <map>
-#include <memory>
 #include <utility>
 #include <cstdint>
 #include <cassert>
@@ -55,16 +54,11 @@ namespace sdbus {
 
 namespace sdbus {
 
-    // Assume the caller has already obtained message ownership
-    struct adopt_message_t { explicit adopt_message_t() = default; };
-    inline constexpr adopt_message_t adopt_message{};
-
     /********************************************//**
      * @class Message
      *
      * Message represents a D-Bus message, which can be either method call message,
-     * method reply message, signal message, or a plain message serving as a storage
+     * method reply message, signal message, or a plain message.
-     * for serialized data.
      *
      * Serialization and deserialization functions are provided for types supported
      * by D-Bus.
@@ -173,17 +167,12 @@ namespace sdbus {
         mutable bool ok_{true};
     };
 
-    struct dont_request_slot_t { explicit dont_request_slot_t() = default; };
-    inline constexpr dont_request_slot_t dont_request_slot{};
-
     class MethodCall : public Message
     {
         using Message::Message;
         friend Factory;
 
     public:
-        using Slot = std::unique_ptr<void, std::function<void(void*)>>;
-
         MethodCall() = default;
 
         MethodReply send(uint64_t timeout) const;
@@ -244,6 +233,7 @@ namespace sdbus {
         PropertyGetReply() = default;
     };
 
+    // Represents any of the above message types, or just a message that serves as a container for data
     class PlainMessage : public Message
     {
         using Message::Message;

include/sdbus-c++/TypeTraits.h

@@ -33,6 +33,7 @@
 #include <map>
 #include <cstdint>
 #include <functional>
+#include <memory>
 #include <tuple>
 
 // Forward declarations
@@ -53,12 +54,30 @@ namespace sdbus {
 
 namespace sdbus {
 
+    // Callbacks from sdbus-c++
     using method_callback = std::function<void(MethodCall msg)>;
     using async_reply_handler = std::function<void(MethodReply& reply, const Error* error)>;
     using signal_handler = std::function<void(Signal& signal)>;
     using property_set_callback = std::function<void(PropertySetCall& msg)>;
     using property_get_callback = std::function<void(PropertyGetReply& reply)>;
 
+    // Type-erased RAII-style handle to callbacks/subscriptions registered to sdbus-c++
+    using Slot = std::unique_ptr<void, std::function<void(void*)>>;
+
+    // Tag specifying that an owning slot handle shall be returned from the function
+    struct request_slot_t { explicit request_slot_t() = default; };
+    inline constexpr request_slot_t request_slot{};
+    // Tag specifying that the library shall own the slot resulting from the call of the function (so-called floating slot)
+    struct dont_request_slot_t { explicit dont_request_slot_t() = default; };
+    inline constexpr dont_request_slot_t dont_request_slot{};
+    // Tag denoting the assumption that the caller has already obtained message ownership
+    struct adopt_message_t { explicit adopt_message_t() = default; };
+    inline constexpr adopt_message_t adopt_message{};
+    // Tag denoting the assumption that the caller has already obtained fd ownership
+    struct adopt_fd_t { explicit adopt_fd_t() = default; };
+    inline constexpr adopt_fd_t adopt_fd{};
+
+    // Template specializations for getting D-Bus signatures from C++ types
     template <typename _T>
     struct signature_of
     {

include/sdbus-c++/Types.h

@@ -185,9 +185,6 @@ namespace sdbus {
         using std::string::operator=;
     };
 
-    struct adopt_fd_t { explicit adopt_fd_t() = default; };
-    inline constexpr adopt_fd_t adopt_fd{};
-
     /********************************************//**
      * @struct UnixFd
      *

src/Connection.cpp

@@ -144,16 +144,16 @@ ISdBus& Connection::getSdBusInterface()
 
 void Connection::addObjectManager(const std::string& objectPath)
 {
-    Connection::addObjectManager(objectPath, nullptr);
+    auto r = iface_->sd_bus_add_object_manager(bus_.get(), nullptr, objectPath.c_str());
+
+    SDBUS_THROW_ERROR_IF(r < 0, "Failed to add object manager", -r);
 }
 
-SlotPtr Connection::addObjectManager(const std::string& objectPath, void* /*dummy*/)
+Slot Connection::addObjectManager(const std::string& objectPath, request_slot_t)
 {
     sd_bus_slot *slot{};
 
-    auto r = iface_->sd_bus_add_object_manager( bus_.get()
+    auto r = iface_->sd_bus_add_object_manager(bus_.get(), &slot, objectPath.c_str());
-                                              , &slot
-                                              , objectPath.c_str() );
 
     SDBUS_THROW_ERROR_IF(r < 0, "Failed to add object manager", -r);
 
@@ -178,10 +178,10 @@ uint64_t Connection::getMethodCallTimeout() const
     return timeout;
 }
 
-SlotPtr Connection::addObjectVTable( const std::string& objectPath
+Slot Connection::addObjectVTable( const std::string& objectPath
-                                   , const std::string& interfaceName
+                                , const std::string& interfaceName
-                                   , const sd_bus_vtable* vtable
+                                , const sd_bus_vtable* vtable
-                                   , void* userData )
+                                , void* userData )
 {
     sd_bus_slot *slot{};
 
@@ -296,12 +296,12 @@ void Connection::emitInterfacesRemovedSignal( const std::string& objectPath
     SDBUS_THROW_ERROR_IF(r < 0, "Failed to emit InterfacesRemoved signal", -r);
 }
 
-SlotPtr Connection::registerSignalHandler( const std::string& sender
+Slot Connection::registerSignalHandler( const std::string& sender
-                                         , const std::string& objectPath
+                                      , const std::string& objectPath
-                                         , const std::string& interfaceName
+                                      , const std::string& interfaceName
-                                         , const std::string& signalName
+                                      , const std::string& signalName
-                                         , sd_bus_message_handler_t callback
+                                      , sd_bus_message_handler_t callback
-                                         , void* userData )
+                                      , void* userData )
 {
     sd_bus_slot *slot{};
 
@@ -457,10 +457,10 @@ bool Connection::waitForNextRequest()
     return true;
 }
 
-    std::string Connection::composeSignalMatchFilter(const std::string &sender,
+std::string Connection::composeSignalMatchFilter( const std::string &sender
-                                                     const std::string &objectPath,
+                                                , const std::string &objectPath
-                                                     const std::string &interfaceName,
+                                                , const std::string &interfaceName
-                                                     const std::string &signalName)
+                                                , const std::string &signalName )
 {
     std::string filter;
 

src/Connection.h

@@ -43,8 +43,7 @@
 namespace sdbus::internal {
 
     class Connection final
-        : public sdbus::IConnection           // External, public interface
+        : public sdbus::internal::IConnection
-        , public sdbus::internal::IConnection // Internal, private interface
     {
     public:
         // Bus type tags
@@ -69,7 +68,7 @@ namespace sdbus::internal {
         bool processPendingRequest() override;
 
         void addObjectManager(const std::string& objectPath) override;
-        SlotPtr addObjectManager(const std::string& objectPath, void* /*dummy*/) override;
+        Slot addObjectManager(const std::string& objectPath, request_slot_t) override;
 
         void setMethodCallTimeout(uint64_t timeout) override;
         uint64_t getMethodCallTimeout() const override;
@@ -77,10 +76,10 @@ namespace sdbus::internal {
         const ISdBus& getSdBusInterface() const override;
         ISdBus& getSdBusInterface() override;
 
-        SlotPtr addObjectVTable( const std::string& objectPath
+        Slot addObjectVTable( const std::string& objectPath
-                               , const std::string& interfaceName
+                            , const std::string& interfaceName
-                               , const sd_bus_vtable* vtable
+                            , const sd_bus_vtable* vtable
-                               , void* userData ) override;
+                            , void* userData ) override;
 
         PlainMessage createPlainMessage() const override;
         MethodCall createMethodCall( const std::string& destination
@@ -101,12 +100,12 @@ namespace sdbus::internal {
         void emitInterfacesRemovedSignal( const std::string& objectPath
                                         , const std::vector<std::string>& interfaces ) override;
 
-        SlotPtr registerSignalHandler( const std::string& sender
+        Slot registerSignalHandler( const std::string& sender
-                                     , const std::string& objectPath
+                                  , const std::string& objectPath
-                                     , const std::string& interfaceName
+                                  , const std::string& interfaceName
-                                     , const std::string& signalName
+                                  , const std::string& signalName
-                                     , sd_bus_message_handler_t callback
+                                  , sd_bus_message_handler_t callback
-                                     , void* userData ) override;
+                                  , void* userData ) override;
 
         MethodReply tryCallMethodSynchronously(const MethodCall& message, uint64_t timeout) override;
 
@@ -118,9 +117,10 @@ namespace sdbus::internal {
         BusPtr openBus(const std::function<int(sd_bus**)>& busFactory);
         void finishHandshake(sd_bus* bus);
         bool waitForNextRequest();
-        static std::string composeSignalMatchFilter(const std::string &sender, const std::string &objectPath,
+        static std::string composeSignalMatchFilter( const std::string &sender
-                                                    const std::string &interfaceName,
+                                                   , const std::string &objectPath
-                                                    const std::string &signalName);
+                                                   , const std::string &interfaceName
+                                                   , const std::string &signalName);
         void notifyEventLoop(int fd) const;
         void notifyEventLoopToExit() const;
         void clearEventLoopNotification(int fd) const;

src/IConnection.h

@@ -27,6 +27,7 @@
 #ifndef SDBUS_CXX_INTERNAL_ICONNECTION_H_
 #define SDBUS_CXX_INTERNAL_ICONNECTION_H_
 
+#include <sdbus-c++/IConnection.h>
 #include <systemd/sd-bus.h>
 #include <string>
 #include <memory>
@@ -46,20 +47,19 @@ namespace sdbus {
 
 namespace sdbus::internal {
 
-    using SlotPtr = std::unique_ptr<void, std::function<void(void*)>>;
-
     class IConnection
+        : public ::sdbus::IConnection
     {
     public:
-        virtual ~IConnection() = default;
+        ~IConnection() override = default;
 
         virtual const ISdBus& getSdBusInterface() const = 0;
         virtual ISdBus& getSdBusInterface() = 0;
 
-        [[nodiscard]] virtual SlotPtr addObjectVTable( const std::string& objectPath
+        [[nodiscard]] virtual Slot addObjectVTable( const std::string& objectPath
-                                                     , const std::string& interfaceName
+                                                  , const std::string& interfaceName
-                                                     , const sd_bus_vtable* vtable
+                                                  , const sd_bus_vtable* vtable
-                                                     , void* userData ) = 0;
+                                                  , void* userData ) = 0;
 
         virtual PlainMessage createPlainMessage() const = 0;
         virtual MethodCall createMethodCall( const std::string& destination
@@ -80,17 +80,16 @@ namespace sdbus::internal {
         virtual void emitInterfacesRemovedSignal( const std::string& objectPath
                                                 , const std::vector<std::string>& interfaces ) = 0;
 
-        [[nodiscard]] virtual SlotPtr addObjectManager(const std::string& objectPath, void* /*dummy*/ = nullptr) = 0;
+        using sdbus::IConnection::addObjectManager;
+        [[nodiscard]] virtual Slot addObjectManager(const std::string& objectPath, request_slot_t) = 0;
 
-        [[nodiscard]] virtual SlotPtr registerSignalHandler( const std::string& sender
+        [[nodiscard]] virtual Slot registerSignalHandler( const std::string& sender
-                                                           , const std::string& objectPath
+                                                        , const std::string& objectPath
-                                                           , const std::string& interfaceName
+                                                        , const std::string& interfaceName
-                                                           , const std::string& signalName
+                                                        , const std::string& signalName
-                                                           , sd_bus_message_handler_t callback
+                                                        , sd_bus_message_handler_t callback
-                                                           , void* userData ) = 0;
+                                                        , void* userData ) = 0;
 
-        virtual void enterEventLoopAsync() = 0;
-        virtual void leaveEventLoop() = 0;
         virtual void notifyEventLoopNewTimeout() const = 0;
         virtual MethodReply tryCallMethodSynchronously(const MethodCall& message, uint64_t timeout) = 0;
     };

src/Message.cpp

@@ -803,7 +803,7 @@ void MethodCall::send(void* callback, void* userData, uint64_t timeout, dont_req
     connection_->notifyEventLoopNewTimeout();
 }
 
-MethodCall::Slot MethodCall::send(void* callback, void* userData, uint64_t timeout) const
+Slot MethodCall::send(void* callback, void* userData, uint64_t timeout) const
 {
     sd_bus_slot* slot;
 
@@ -814,7 +814,7 @@ MethodCall::Slot MethodCall::send(void* callback, void* userData, uint64_t timeo
     SDBUS_THROW_ERROR_IF(connection_ == nullptr, "Invalid use of MethodCall API", ENOTSUP);
     connection_->notifyEventLoopNewTimeout();
 
-    return Slot{slot, [sdbus_ = sdbus_](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }};
+    return {slot, [sdbus_ = sdbus_](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }};
 }
 
 MethodReply MethodCall::createReply() const

src/Object.cpp

@@ -217,7 +217,7 @@ void Object::emitInterfacesRemovedSignal(const std::vector<std::string>& interfa
 
 void Object::addObjectManager()
 {
-    objectManagerSlot_ = connection_.addObjectManager(objectPath_);
+    objectManagerSlot_ = connection_.addObjectManager(objectPath_, request_slot);
 }
 
 void Object::removeObjectManager()
@@ -232,7 +232,7 @@ bool Object::hasObjectManager() const
 
 sdbus::IConnection& Object::getConnection() const
 {
-    return dynamic_cast<sdbus::IConnection&>(connection_);
+    return connection_;
 }
 
 const std::string& Object::getObjectPath() const

src/Object.h

@@ -144,7 +144,7 @@ namespace sdbus::internal {
 
             // This is intentionally the last member, because it must be destructed first,
             // releasing callbacks above before the callbacks themselves are destructed.
-            SlotPtr slot;
+            Slot slot;
         };
 
         InterfaceData& getInterface(const std::string& interfaceName);
@@ -177,7 +177,7 @@ namespace sdbus::internal {
         sdbus::internal::IConnection& connection_;
         std::string objectPath_;
         std::map<InterfaceName, InterfaceData> interfaces_;
-        SlotPtr objectManagerSlot_;
+        Slot objectManagerSlot_;
         std::atomic<const Message*> m_CurrentlyProcessedMessage{nullptr};
     };
 

src/Proxy.cpp

@@ -219,7 +219,7 @@ void Proxy::unregister()
 
 sdbus::IConnection& Proxy::getConnection() const
 {
-    return dynamic_cast<sdbus::IConnection&>(*connection_);
+    return *connection_;
 }
 
 const std::string& Proxy::getObjectPath() const

src/Proxy.h

@@ -103,18 +103,18 @@ namespace sdbus::internal {
             using SignalName = std::string;
             struct SignalData
             {
-                SignalData(Proxy& proxy, signal_handler callback, SlotPtr slot)
+                SignalData(Proxy& proxy, signal_handler callback, Slot slot)
                     : proxy(proxy)
                     , callback(std::move(callback))
                     , slot(std::move(slot))
                 {}
                 Proxy& proxy;
                 signal_handler callback;
-                // slot_ must be listed after callback_ to ensure that slot_ is destructed first.
+                // slot must be listed after callback to ensure that slot is destructed first.
-                // Destructing the slot_ will sd_bus_slot_unref() the callback.
+                // Destructing the slot will sd_bus_slot_unref() the callback.
                 // Only after sd_bus_slot_unref(), we can safely delete the callback. The bus mutex (SdBus::sdbusMutex_)
                 // ensures that sd_bus_slot_unref() and the callback execute sequentially.
-                SlotPtr slot;
+                Slot slot;
             };
             std::map<SignalName, std::unique_ptr<SignalData>> signals_;
         };
@@ -131,7 +131,7 @@ namespace sdbus::internal {
             {
                 Proxy& proxy;
                 async_reply_handler callback;
-                MethodCall::Slot slot;
+                Slot slot;
             };
 
             ~AsyncCalls()