[SSSD] [PATCH] libsss_dbus

Sumit Bose sbose at redhat.com
Thu May 22 09:17:18 UTC 2014


On Thu, May 22, 2014 at 11:07:51AM +0200, Sumit Bose wrote:
> On Mon, May 19, 2014 at 10:40:40PM +0200, Pavel Březina wrote:
> > On 05/16/2014 03:32 PM, Jakub Hrozek wrote:
> > >On Fri, May 16, 2014 at 02:54:55PM +0200, Pavel Březina wrote:
> > >>On 05/16/2014 02:04 PM, Jakub Hrozek wrote:
> > >>>On Thu, May 15, 2014 at 09:23:46PM +0200, Pavel Březina wrote:
> > >>>>On 05/15/2014 09:09 PM, Pavel Březina wrote:
> > 
> 
> ...
> 
> 
> > All concerns should be fixed (but the memory leak in test). I also changed
> > the prefix to sss_sifp as Sumit asked (sending as separate patch).
> 
> Thank you, imo it more looks much more consistent.
> 
> Please find attached a small patch which should illustrate my other
> suggestion to remove the build time DBus dependency for the user of the
> library. Doc fixes and a free function for the new struct are still
> missing, the patch should just illustrate the idea. If you think this
> makes sense, I can add the missing parts if you like.
> 
> With this the dbus-1 requirement in the pc file can be removed as well
> because the runtime dependency for DBus will be solved by package
> management. Btw, I think there is an issue in the current pc file, both
> requirements should be in a single line.

and now with the patch ...

> 
> bye,
> Sumit
> 
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> 
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
-------------- next part --------------
>From bb3134570d21a1a536b57885583c70c8857bec08 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 20 May 2014 14:53:04 +0200
Subject: [PATCH] sifp change

---
 src/lib/sifp/sss_sifp.c         | 117 ++++++++++++++++++++++++++++------------
 src/lib/sifp/sss_sifp.h         |  16 +++---
 src/lib/sifp/sss_sifp_private.h |   1 +
 3 files changed, 93 insertions(+), 41 deletions(-)

diff --git a/src/lib/sifp/sss_sifp.c b/src/lib/sifp/sss_sifp.c
index 42f5976..e3000e6 100644
--- a/src/lib/sifp/sss_sifp.c
+++ b/src/lib/sifp/sss_sifp.c
@@ -27,6 +27,11 @@
 
 #define DBUS_IFACE_PROP "org.freedesktop.DBus.Properties"
 
+
+struct sss_sifp_dbus_message {
+    DBusMessage *dbus_message;
+};
+
 static void * default_alloc(size_t size, void *pvt)
 {
     return malloc(size);
@@ -37,16 +42,23 @@ static void default_free(void *ptr, void *pvt)
     free(ptr);
 }
 
-static DBusMessage * sss_sifp_create_ifp_msg(const char *method)
+static DBusMessage * sss_sifp_create_ifp_msg(sss_sifp_ctx *ctx,
+                                             const char *method)
 {
-    return sss_sifp_create_message(SSS_DBUS_PATH_IFP, SSS_DBUS_IFACE_IFP,
-                                   method);
+    sss_sifp_dbus_message *msg;
+    msg = sss_sifp_create_message(ctx, SSS_DBUS_PATH_IFP, SSS_DBUS_IFACE_IFP,
+                                  method);
+    return msg->dbus_message;
 }
 
-static DBusMessage * sss_sifp_create_prop_msg(const char *object_path,
+static DBusMessage * sss_sifp_create_prop_msg(sss_sifp_ctx *ctx,
+                                              const char *object_path,
                                               const char *method)
 {
-    return sss_sifp_create_message(object_path, DBUS_IFACE_PROP, method);
+    sss_sifp_dbus_message *msg;
+    msg = sss_sifp_create_message(ctx, object_path, DBUS_IFACE_PROP, method);
+
+    return msg->dbus_message;
 }
 
 static sss_sifp_error sss_sifp_ifp_call(sss_sifp_ctx *ctx,
@@ -55,27 +67,33 @@ static sss_sifp_error sss_sifp_ifp_call(sss_sifp_ctx *ctx,
                                         va_list ap,
                                         DBusMessage **_reply)
 {
-   DBusMessage *msg = NULL;
-   sss_sifp_error ret;
+    sss_sifp_dbus_message msg = {NULL};
+    sss_sifp_dbus_message *reply;
+    sss_sifp_error ret;
 
-   msg = sss_sifp_create_ifp_msg(method);
-   if (msg == NULL) {
-       ret = SSS_SIFP_OUT_OF_MEMORY;
-       goto done;
-   }
+    msg.dbus_message = sss_sifp_create_ifp_msg(ctx, method);
+    if (msg.dbus_message == NULL) {
+        ret = SSS_SIFP_OUT_OF_MEMORY;
+        goto done;
+    }
 
-   if (first_arg_type != DBUS_TYPE_INVALID) {
-       dbus_message_append_args_valist(msg, first_arg_type, ap);
-   }
+    if (first_arg_type != DBUS_TYPE_INVALID) {
+        dbus_message_append_args_valist(msg.dbus_message, first_arg_type, ap);
+    }
 
-   ret = sss_sifp_send_message(ctx, msg, _reply);
+    ret = sss_sifp_send_message(ctx, &msg, &reply);
+    if (ret != SSS_SIFP_OK) {
+        goto done;
+    }
+    *_reply = reply->dbus_message;
+    _free(ctx, reply);
 
 done:
-   if (msg != NULL) {
-       dbus_message_unref(msg);
-   }
+    if (msg.dbus_message != NULL) {
+        dbus_message_unref(msg.dbus_message);
+    }
 
-   return ret;
+    return ret;
 }
 
 sss_sifp_error
@@ -168,32 +186,46 @@ sss_sifp_get_last_io_error_message(sss_sifp_ctx *ctx)
     return ctx->io_error->message;
 }
 
-DBusMessage *
-sss_sifp_create_message(const char *object_path,
+sss_sifp_dbus_message *
+sss_sifp_create_message(sss_sifp_ctx *ctx,
+                        const char *object_path,
                         const char *interface,
                         const char *method)
 {
-    return dbus_message_new_method_call(SSS_DBUS_IFP, object_path,
-                                        interface, method);
+    sss_sifp_dbus_message *msg;
+
+    msg = _alloc_zero(ctx,sss_sifp_dbus_message, 1);
+    if (msg == NULL) {
+        return NULL;
+    }
+
+    msg->dbus_message = dbus_message_new_method_call(SSS_DBUS_IFP, object_path,
+                                                    interface, method);
+    if (msg->dbus_message == NULL) {
+        return NULL;
+    }
+
+    return msg;
 }
 
 sss_sifp_error
 sss_sifp_send_message(sss_sifp_ctx *ctx,
-                      DBusMessage *msg,
-                      DBusMessage **_reply)
+                      sss_sifp_dbus_message *msg,
+                      sss_sifp_dbus_message **_reply)
 {
     return sss_sifp_send_message_ex(ctx, msg, 5000, _reply);
 }
 
 sss_sifp_error
 sss_sifp_send_message_ex(sss_sifp_ctx *ctx,
-                         DBusMessage *msg,
+                         sss_sifp_dbus_message *msg,
                          int timeout,
-                         DBusMessage **_reply)
+                         sss_sifp_dbus_message **_reply)
 {
     DBusMessage *reply = NULL;
     DBusError dbus_error;
     sss_sifp_error ret;
+    sss_sifp_dbus_message *sss_repl;
 
     if (ctx == NULL || msg == NULL) {
         return SSS_SIFP_INVALID_ARGUMENT;
@@ -201,7 +233,8 @@ sss_sifp_send_message_ex(sss_sifp_ctx *ctx,
 
     dbus_error_init(&dbus_error);
 
-    reply = dbus_connection_send_with_reply_and_block(ctx->conn, msg,
+    reply = dbus_connection_send_with_reply_and_block(ctx->conn,
+                                                      msg->dbus_message,
                                                       timeout, &dbus_error);
     if (dbus_error_is_set(&dbus_error)) {
         sss_sifp_set_io_error(ctx, &dbus_error);
@@ -212,7 +245,13 @@ sss_sifp_send_message_ex(sss_sifp_ctx *ctx,
     if (_reply == NULL) {
         dbus_message_unref(reply);
     } else {
-        *_reply = reply;
+        sss_repl = _alloc_zero(ctx, sss_sifp_dbus_message, 1);
+        if (sss_repl == NULL) {
+            ret = SSS_SIFP_OUT_OF_MEMORY;
+            goto done;
+        }
+        sss_repl->dbus_message = reply;
+        *_reply = sss_repl;
     }
 
     ret = SSS_SIFP_OK;
@@ -231,6 +270,8 @@ sss_sifp_fetch_attr(sss_sifp_ctx *ctx,
 {
     DBusMessage *msg = NULL;
     DBusMessage *reply = NULL;
+    sss_sifp_dbus_message sifp_msg = {NULL};
+    sss_sifp_dbus_message *sifp_reply;
     dbus_bool_t bret;
     sss_sifp_error ret;
 
@@ -245,7 +286,7 @@ sss_sifp_fetch_attr(sss_sifp_ctx *ctx,
      * Out: variant(misc:value)
      */
 
-    msg = sss_sifp_create_prop_msg(object_path, "Get");
+    msg = sss_sifp_create_prop_msg(ctx, object_path, "Get");
     if (msg == NULL) {
         ret = SSS_SIFP_OUT_OF_MEMORY;
         goto done;
@@ -259,10 +300,13 @@ sss_sifp_fetch_attr(sss_sifp_ctx *ctx,
         goto done;
     }
 
-    ret = sss_sifp_send_message(ctx, msg, &reply);
+    sifp_msg.dbus_message = msg;
+    ret = sss_sifp_send_message(ctx, &sifp_msg, &sifp_reply);
     if (ret != SSS_SIFP_OK) {
         goto done;
     }
+    reply = sifp_reply->dbus_message;
+    _free(ctx, sifp_reply);
 
     ret = sss_sifp_parse_attr(ctx, name, reply, _attrs);
 
@@ -286,6 +330,8 @@ sss_sifp_fetch_all_attrs(sss_sifp_ctx *ctx,
 {
     DBusMessage *msg = NULL;
     DBusMessage *reply = NULL;
+    sss_sifp_dbus_message sifp_msg = {NULL};
+    sss_sifp_dbus_message *sifp_reply;
     dbus_bool_t bret;
     sss_sifp_error ret;
 
@@ -294,7 +340,7 @@ sss_sifp_fetch_all_attrs(sss_sifp_ctx *ctx,
         return SSS_SIFP_INVALID_ARGUMENT;
     }
 
-    msg = sss_sifp_create_prop_msg(object_path, "GetAll");
+    msg = sss_sifp_create_prop_msg(ctx, object_path, "GetAll");
     if (msg == NULL) {
         ret = SSS_SIFP_OUT_OF_MEMORY;
         goto done;
@@ -307,10 +353,13 @@ sss_sifp_fetch_all_attrs(sss_sifp_ctx *ctx,
         goto done;
     }
 
-    ret = sss_sifp_send_message(ctx, msg, &reply);
+    sifp_msg.dbus_message = msg;
+    ret = sss_sifp_send_message(ctx, &sifp_msg, &sifp_reply);
     if (ret != SSS_SIFP_OK) {
         goto done;
     }
+    reply = sifp_reply->dbus_message;
+    _free(ctx, sifp_reply);
 
     ret = sss_sifp_parse_attr_list(ctx, reply, _attrs);
 
diff --git a/src/lib/sifp/sss_sifp.h b/src/lib/sifp/sss_sifp.h
index 064df5f..657cb17 100644
--- a/src/lib/sifp/sss_sifp.h
+++ b/src/lib/sifp/sss_sifp.h
@@ -24,7 +24,6 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include <unistd.h>
-#include <dbus/dbus.h>
 #include <dhash.h>
 
 /**
@@ -53,6 +52,8 @@
 #define SSS_DBUS_IFACE_USERS "org.freedesktop.sssd.infopipe.Users"
 #define SSS_DBUS_IFACE_GROUPS "org.freedesktop.sssd.infopipe.Groups"
 
+
+typedef struct sss_sifp_dbus_message sss_sifp_dbus_message;
 /**
  * Opaque libsss_sifp context. One context shall not be used by multiple
  * threads. Each thread needs to create and use its own context.
@@ -173,8 +174,9 @@ sss_sifp_get_last_io_error_message(sss_sifp_ctx *ctx);
  *
  * @return D-Bus message.
  */
-DBusMessage *
-sss_sifp_create_message(const char *object_path,
+sss_sifp_dbus_message *
+sss_sifp_create_message(sss_sifp_ctx *ctx,
+                        const char *object_path,
                         const char *interface,
                         const char *method);
 
@@ -192,8 +194,8 @@ sss_sifp_create_message(const char *object_path,
  */
 sss_sifp_error
 sss_sifp_send_message(sss_sifp_ctx *ctx,
-                      DBusMessage *msg,
-                      DBusMessage **_reply);
+                      sss_sifp_dbus_message *msg,
+                      sss_sifp_dbus_message **_reply);
 
 /**
  * @brief Send D-Bus message to SSSD InfoPipe bus.
@@ -207,9 +209,9 @@ sss_sifp_send_message(sss_sifp_ctx *ctx,
  */
 sss_sifp_error
 sss_sifp_send_message_ex(sss_sifp_ctx *ctx,
-                         DBusMessage *msg,
+                         sss_sifp_dbus_message *msg,
                          int timeout,
-                         DBusMessage **_reply);
+                         sss_sifp_dbus_message **_reply);
 
 /**
  * @brief Fetch selected attributes of given object.
diff --git a/src/lib/sifp/sss_sifp_private.h b/src/lib/sifp/sss_sifp_private.h
index 1798791..29ad4b7 100644
--- a/src/lib/sifp/sss_sifp_private.h
+++ b/src/lib/sifp/sss_sifp_private.h
@@ -23,6 +23,7 @@
 
 #define SSS_DBUS_PATH_IFP "/org/freedesktop/sssd/infopipe"
 
+#include <dbus/dbus.h>
 #include <dhash.h>
 #include "lib/sifp/sss_sifp.h"
 
-- 
1.8.3.1



More information about the sssd-devel mailing list