sbus: Refactor how we export DBus interfaces

Most importantly, stop using per connection private data. This doesn't
scale when you have more than one thing exporting or exported on a
connection.

Remove struct sbus_interface and expand sbus_conn_add_interface()
function. Remove various struct sbus_interface args to connection
initialization functions and make callers use sbus_conn_add_interface()
directly. The old method was optimized for exporting one interface
on a connection. We'll have connections that export zero, one or more
interfaces.

To export an interface on a DBus server, call sbus_conn_add_interface()
from within the sbus_server_conn_init_fn. To export an interface on
a DBus client, call sbus_conn_add_interface() after sbus_new_connection()
returns.

As before struct sbus_interface represents an object exported via DBus.
However it is now talloc allocated. One can set instance data on the
struct sbus_interface. This instance data is passed to the various
handlers and used in their implementation.

However, we now have type safe interface exporting in the various
high level sss_process_init() sss_monitor_init() and so on.

Introspection support was not in use, and is now gone until we
implement it using the metadata (future patch).

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>
Reviewed-by: Pavel Březina <pbrezina@redhat.com>
Reviewed-by: Lukáš Slebodník <lslebodn@redhat.com>
(cherry picked from commit 07e941c1bbdc752142bbd3b838c540bc7ecd0ed7)

6 files changed, 54 insertions, 102 deletions

diff --git a/src/sbus/sbus_client.c b/src/sbus/sbus_client.c
index f516555a7..ce862534d 100644
--- a/src/sbus/sbus_client.c
+++ b/src/sbus/sbus_client.c
@@ -26,10 +26,7 @@
 int sbus_client_init(TALLOC_CTX *mem_ctx,
                      struct tevent_context *ev,
                      const char *server_address,
-                     struct sbus_interface *intf,
-                     struct sbus_connection **_conn,
-                     sbus_conn_destructor_fn destructor,
-                     void *conn_pvt_data)
+                     struct sbus_connection **_conn)
 {
     struct sbus_connection *conn = NULL;
     int ret;
@@ -53,15 +50,11 @@ int sbus_client_init(TALLOC_CTX *mem_ctx,
         return EIO;
     }
 
-    ret = sbus_new_connection(mem_ctx, ev, server_address, intf, &conn);
+    ret = sbus_new_connection(mem_ctx, ev, server_address, &conn);
     if (ret != EOK) {
         goto fail;
     }
 
-    /* Set connection destructor and private data */
-    sbus_conn_set_destructor(conn, destructor);
-    sbus_conn_set_private_data(conn, conn_pvt_data);
-
     *_conn = conn;
     return EOK;
 
diff --git a/src/sbus/sbus_client.h b/src/sbus/sbus_client.h
index 742f8a10c..aea1342ea 100644
--- a/src/sbus/sbus_client.h
+++ b/src/sbus/sbus_client.h
@@ -28,9 +28,6 @@
 int sbus_client_init(TALLOC_CTX *mem_ctx,
                      struct tevent_context *ev,
                      const char *server_address,
-                     struct sbus_interface *intf,
-                     struct sbus_connection **_conn,
-                     sbus_conn_destructor_fn destructor,
-                     void *conn_pvt_data);
+                     struct sbus_connection **_conn);
 
 #endif /* SBUS_CLIENT_H_ */
diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h
index dcb669079..ef728bd30 100644
--- a/src/sbus/sssd_dbus.h
+++ b/src/sbus/sssd_dbus.h
@@ -23,12 +23,13 @@
 #define _SSSD_DBUS_H_
 
 struct sbus_connection;
-
+struct sbus_interface;
 struct sbus_request;
 
 #include <dbus/dbus.h>
 
-typedef int (*sbus_msg_handler_fn)(struct sbus_request *dbus_req);
+typedef int (*sbus_msg_handler_fn)(struct sbus_request *dbus_req,
+                                   void *instance_data);
 
 /*
  * sbus_conn_destructor_fn
@@ -79,14 +80,28 @@ struct sbus_vtable {
 struct sbus_interface {
     const char *path;
     struct sbus_vtable *vtable;
-    sbus_msg_handler_fn introspect_fn;
+    void *instance_data;
 };
 
+/*
+ * Creates a new struct sbus_interface instance to be exported by a DBus
+ * service.
+ *
+ * Pass the result to sbus_conn_add_interface(). The interface
+ * will be exported at @object_path. The method handlers are represented by
+ * @iface_vtable. @instance_data contains additional caller specific data
+ * which is made available to handlers.
+ */
+struct sbus_interface *
+sbus_new_interface(TALLOC_CTX *mem_ctx,
+                   const char *object_path,
+                   struct sbus_vtable *iface_vtable,
+                   void *instance_data);
+
 /* Server Functions */
 int sbus_new_server(TALLOC_CTX *mem_ctx,
                     struct tevent_context *ev,
                     const char *address,
-                    struct sbus_interface *intf,
                     bool use_symlink,
                     struct sbus_connection **server,
                     sbus_server_conn_init_fn init_fn, void *init_pvt_data);
@@ -103,7 +118,6 @@ int sbus_new_server(TALLOC_CTX *mem_ctx,
 int sbus_new_connection(TALLOC_CTX *ctx,
                         struct tevent_context *ev,
                         const char *address,
-                        struct sbus_interface *intf,
                         struct sbus_connection **conn);
 
 /* sbus_add_connection
@@ -121,19 +135,11 @@ int sbus_new_connection(TALLOC_CTX *ctx,
 int sbus_init_connection(TALLOC_CTX *ctx,
                          struct tevent_context *ev,
                          DBusConnection *dbus_conn,
-                         struct sbus_interface *intf,
                          int connection_type,
                          struct sbus_connection **_conn);
 
-void sbus_conn_set_destructor(struct sbus_connection *conn,
-                              sbus_conn_destructor_fn destructor);
-
-int sbus_default_connection_destructor(void *ctx);
-
 DBusConnection *sbus_get_connection(struct sbus_connection *conn);
 void sbus_disconnect(struct sbus_connection *conn);
-void sbus_conn_set_private_data(struct sbus_connection *conn, void *pvt_data);
-void *sbus_conn_get_private_data(struct sbus_connection *conn);
 int sbus_conn_add_interface(struct sbus_connection *conn,
                             struct sbus_interface *intf);
 bool sbus_conn_disconnecting(struct sbus_connection *conn);
diff --git a/src/sbus/sssd_dbus_connection.c b/src/sbus/sssd_dbus_connection.c
index 6535bc90e..1d927c847 100644
--- a/src/sbus/sssd_dbus_connection.c
+++ b/src/sbus/sssd_dbus_connection.c
@@ -159,7 +159,6 @@ static int sbus_conn_set_fns(struct sbus_connection *conn);
 int sbus_init_connection(TALLOC_CTX *ctx,
                          struct tevent_context *ev,
                          DBusConnection *dbus_conn,
-                         struct sbus_interface *intf,
                          int connection_type,
                          struct sbus_connection **_conn)
 {
@@ -174,12 +173,6 @@ int sbus_init_connection(TALLOC_CTX *ctx,
     conn->dbus.conn = dbus_conn;
     conn->connection_type = connection_type;
 
-    ret = sbus_conn_add_interface(conn, intf);
-    if (ret != EOK) {
-        talloc_free(conn);
-        return ret;
-    }
-
     ret = sbus_conn_set_fns(conn);
     if (ret != EOK) {
         talloc_free(conn);
@@ -194,13 +187,6 @@ static int sbus_conn_set_fns(struct sbus_connection *conn)
 {
     dbus_bool_t dbret;
 
-    /*
-     * Set the default destructor
-     * Connections can override this with
-     * sbus_conn_set_destructor
-     */
-    sbus_conn_set_destructor(conn, NULL);
-
     /* Set up DBusWatch functions */
     dbret = dbus_connection_set_watch_functions(conn->dbus.conn,
                                                 sbus_add_watch,
@@ -244,8 +230,7 @@ static int sbus_conn_set_fns(struct sbus_connection *conn)
 }
 
 int sbus_new_connection(TALLOC_CTX *ctx, struct tevent_context *ev,
-                        const char *address, struct sbus_interface *intf,
-                        struct sbus_connection **_conn)
+                        const char *address, struct sbus_connection **_conn)
 {
     struct sbus_connection *conn;
     DBusConnection *dbus_conn;
@@ -264,8 +249,7 @@ int sbus_new_connection(TALLOC_CTX *ctx, struct tevent_context *ev,
         return EIO;
     }
 
-    ret = sbus_init_connection(ctx, ev, dbus_conn, intf,
-                               SBUS_CONN_TYPE_SHARED, &conn);
+    ret = sbus_init_connection(ctx, ev, dbus_conn, SBUS_CONN_TYPE_SHARED, &conn);
     if (ret != EOK) {
         /* FIXME: release resources */
     }
@@ -279,26 +263,7 @@ int sbus_new_connection(TALLOC_CTX *ctx, struct tevent_context *ev,
     return ret;
 }
 
-/*
- * sbus_conn_set_destructor
- * Configures a callback to clean up this connection when it
- * is finalized.
- * @param conn The sbus_connection created
- * when this connection was established
- * @param destructor The destructor function that should be
- * called when the connection is finalized. If passed NULL,
- * this will reset the connection to the default destructor.
- */
-void sbus_conn_set_destructor(struct sbus_connection *conn,
-                              sbus_conn_destructor_fn destructor)
-{
-    if (!conn) return;
-
-    conn->destructor = destructor;
-    /* TODO: Should we try to handle the talloc_destructor too? */
-}
-
-int sbus_default_connection_destructor(void *ctx)
+static int connection_destructor(void *ctx)
 {
     struct sbus_connection *conn;
     conn = talloc_get_type(ctx, struct sbus_connection);
@@ -350,11 +315,6 @@ void sbus_disconnect (struct sbus_connection *conn)
 
     conn->disconnect = 1;
 
-    /* Invoke the custom destructor, if it exists */
-    if (conn->destructor) {
-        conn->destructor(conn);
-    }
-
     /* Unregister object paths */
     sbus_unreg_object_paths(conn);
 
@@ -376,7 +336,7 @@ void sbus_disconnect (struct sbus_connection *conn)
                                              NULL, NULL, NULL);
 
     /* Finalize the connection */
-    sbus_default_connection_destructor(conn);
+    connection_destructor(conn);
 
     dbus_connection_unref(conn->dbus.conn);
     /* Unreferenced conn->dbus_conn *
@@ -465,16 +425,6 @@ DBusHandlerResult sbus_message_handler(DBusConnection *dbus_conn,
             result = DBUS_HANDLER_RESULT_HANDLED;
         }
     }
-    else {
-        /* Special case: check for Introspection request
-         * This is usually only useful for system bus connections
-         */
-        if (strcmp(msg_interface, DBUS_INTROSPECT_INTERFACE) == 0 &&
-            strcmp(msg_method, DBUS_INTROSPECT_METHOD) == 0)
-        {
-            handler_fn = intf_p->intf->introspect_fn;
-        }
-    }
 
     if (handler_fn) {
         dbus_req = sbus_new_request(intf_p->conn, intf_p->intf, message);
@@ -482,7 +432,7 @@ DBusHandlerResult sbus_message_handler(DBusConnection *dbus_conn,
             ret = ENOMEM;
         } else {
             dbus_req->method = method;
-            ret = handler_fn(dbus_req);
+            ret = handler_fn(dbus_req, intf_p->intf->instance_data);
         }
         if (ret != EOK) {
             if (dbus_req)
@@ -496,6 +446,32 @@ DBusHandlerResult sbus_message_handler(DBusConnection *dbus_conn,
     return result;
 }
 
+struct sbus_interface *
+sbus_new_interface(TALLOC_CTX *mem_ctx,
+                   const char *object_path,
+                   struct sbus_vtable *iface_vtable,
+                   void *instance_data)
+{
+    struct sbus_interface *intf;
+
+    intf = talloc_zero(mem_ctx, struct sbus_interface);
+    if (intf == NULL) {
+        DEBUG(SSSDBG_FATAL_FAILURE, "Cannot allocate a new sbus_interface.\n");
+        return NULL;
+    }
+
+    intf->path = talloc_strdup(intf, object_path);
+    if (intf->path == NULL) {
+        DEBUG(SSSDBG_FATAL_FAILURE, "Cannot duplicate object path.\n");
+        talloc_free(intf);
+        return NULL;
+    }
+
+    intf->vtable = iface_vtable;
+    intf->instance_data = instance_data;
+    return intf;
+}
+
 /* Adds a new D-BUS path message handler to the connection
  * Note: this must be a unique path.
  */
@@ -569,16 +545,6 @@ static void sbus_unreg_object_paths(struct sbus_connection *conn)
     }
 }
 
-void sbus_conn_set_private_data(struct sbus_connection *conn, void *pvt_data)
-{
-    conn->pvt_data = pvt_data;
-}
-
-void *sbus_conn_get_private_data(struct sbus_connection *conn)
-{
-    return conn->pvt_data;
-}
-
 static void sbus_reconnect(struct tevent_context *ev,
                            struct tevent_timer *te,
                            struct timeval tv, void *data)
diff --git a/src/sbus/sssd_dbus_private.h b/src/sbus/sssd_dbus_private.h
index eca5c8380..88230cfca 100644
--- a/src/sbus/sssd_dbus_private.h
+++ b/src/sbus/sssd_dbus_private.h
@@ -44,9 +44,6 @@ struct sbus_connection {
     int connection_type;
     int disconnect;
 
-    sbus_conn_destructor_fn destructor;
-    void *pvt_data; /* Private data for this connection */
-
     /* dbus tables and handlers */
     struct sbus_interface_p *intf_list;
 
@@ -59,7 +56,6 @@ struct sbus_connection {
 
     /* server related stuff */
     char *symlink;
-    struct sbus_interface *server_intf;
     sbus_server_conn_init_fn srv_init_fn;
     void *srv_init_data;
 
diff --git a/src/sbus/sssd_dbus_server.c b/src/sbus/sssd_dbus_server.c
index 6973c8f28..5ab4ad0bb 100644
--- a/src/sbus/sssd_dbus_server.c
+++ b/src/sbus/sssd_dbus_server.c
@@ -52,8 +52,7 @@ static void sbus_server_init_new_connection(DBusServer *dbus_server,
     }
 
     DEBUG(SSSDBG_FUNC_DATA,"Adding connection %p.\n", dbus_conn);
-    ret = sbus_init_connection(server, server->ev,
-                               dbus_conn, server->server_intf,
+    ret = sbus_init_connection(server, server->ev, dbus_conn,
                                SBUS_CONN_TYPE_PRIVATE, &conn);
     if (ret != 0) {
         dbus_connection_close(dbus_conn);
@@ -67,9 +66,6 @@ static void sbus_server_init_new_connection(DBusServer *dbus_server,
 
     /*
      * Initialize connection-specific features
-     * This may set a more detailed destructor, but
-     * the default destructor will always be chained
-     * to handle connection cleanup.
      * This function (or its callbacks) should also
      * set up connection-specific methods.
      */
@@ -185,7 +181,6 @@ remove_socket_symlink(const char *symlink_name)
 int sbus_new_server(TALLOC_CTX *mem_ctx,
                     struct tevent_context *ev,
                     const char *address,
-                    struct sbus_interface *intf,
                     bool use_symlink,
                     struct sbus_connection **_server,
                     sbus_server_conn_init_fn init_fn,
@@ -285,7 +280,6 @@ int sbus_new_server(TALLOC_CTX *mem_ctx,
     server->ev = ev;
     server->type = SBUS_SERVER;
     server->dbus.server = dbus_server;
-    server->server_intf = intf;
     server->srv_init_fn = init_fn;
     server->srv_init_data = init_pvt_data;