From 420e47f6a0e173e774faa426d172c6e2160b8302 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina@redhat.com>

Date: Wed, 29 Jun 2016 12:35:59 +0200

Subject: [PATCH 093/102] sbus: add utility function to simplify message and

 reply handling

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

This patch adds the ability to hook DBusMessage to a talloc context

to remove the need of calling dbus_message_unref(). It also provides

an automatical way to detect error in a reply so the caller does

not need to parse it manually and the whole code around DBusError

can be avoided.

Reviewed-by: Lukáš Slebodník <lslebodn@redhat.com>

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>

(cherry picked from commit 439e08cdc5c83b3e5835cb0435983f1da2ffbaf1)

---

 Makefile.am                                      |   2 +

 src/responder/common/data_provider/rdp_message.c |  85 ++-------

 src/sbus/sssd_dbus.h                             |   2 +

 src/sbus/sssd_dbus_utils.c                       | 226 +++++++++++++++++++++++

 src/sbus/sssd_dbus_utils.h                       |  64 +++++++

 src/tools/sssctl/sssctl_domains.c                |  32 +---

 6 files changed, 313 insertions(+), 98 deletions(-)

 create mode 100644 src/sbus/sssd_dbus_utils.c

 create mode 100644 src/sbus/sssd_dbus_utils.h

diff --git a/Makefile.am b/Makefile.am

index ee9b48c666a44781b582ba5d83102b705e898f29..1837e36da7302cb51c0b90e51b762ce0a87cd65f 100644

--- a/Makefile.am

+++ b/Makefile.am

@@ -634,6 +634,7 @@ dist_noinst_HEADERS = \

     src/sbus/sssd_dbus_private.h \

     src/sbus/sssd_dbus_invokers.h \

     src/sbus/sssd_dbus_errors.h \

+    src/sbus/sssd_dbus_utils.h \

     src/db/sysdb.h \

     src/db/sysdb_sudo.h \

     src/db/sysdb_autofs.h \

@@ -915,6 +916,7 @@ libsss_util_la_SOURCES = \

     src/sbus/sssd_dbus_server.c \

     src/sbus/sssd_dbus_signals.c \

     src/sbus/sssd_dbus_common_signals.c \

+    src/sbus/sssd_dbus_utils.c \

     src/util/util.c \

     src/util/memory.c \

     src/util/safe-format-string.c \

diff --git a/src/responder/common/data_provider/rdp_message.c b/src/responder/common/data_provider/rdp_message.c

index e226401567e4a1b2b9784a9aba21540ff5f0bc8d..6ad2ba056e992cd89b87b478d422d1a4259a12d9 100644

--- a/src/responder/common/data_provider/rdp_message.c

+++ b/src/responder/common/data_provider/rdp_message.c

@@ -26,33 +26,6 @@

 #include "sbus/sssd_dbus_errors.h"

 #include "util/util.h"

-static errno_t rdp_error_to_errno(DBusError *error)

-{

-    static struct {

-        const char *name;

-        errno_t ret;

-    } list[] = {{SBUS_ERROR_INTERNAL, ERR_INTERNAL},

-                {SBUS_ERROR_NOT_FOUND, ENOENT},

-                {SBUS_ERROR_DP_FATAL, ERR_TERMINATED},

-                {SBUS_ERROR_DP_OFFLINE, ERR_OFFLINE},

-                {SBUS_ERROR_DP_NOTSUP, ENOTSUP},

-                {NULL, ERR_INTERNAL}

-    };

-    int i;

-

-    if (!dbus_error_is_set(error)) {

-        return EOK;

-    }

-

-    for (i = 0; list[i].name != NULL; i ++) {

-        if (dbus_error_has_name(error, list[i].name)) {

-            return list[i].ret;

-        }

-    }

-

-    return EIO;

-}

-

 static errno_t

 rdp_message_send_internal(struct resp_ctx *rctx,

                           struct sss_domain_info *domain,

@@ -110,7 +83,8 @@ done:

     return ret;

 }

-static errno_t rdp_process_pending_call(DBusPendingCall *pending,

+static errno_t rdp_process_pending_call(TALLOC_CTX *mem_ctx,

+                                        DBusPendingCall *pending,

                                         DBusMessage **_reply)

 {

     DBusMessage *reply;

@@ -130,6 +104,11 @@ static errno_t rdp_process_pending_call(DBusPendingCall *pending,

         goto done;

     }

+    ret = sbus_talloc_bound_message(mem_ctx, reply);

+    if (ret != EOK) {

+        return ret;

+    }

+

     switch (dbus_message_get_type(reply)) {

     case DBUS_MESSAGE_TYPE_METHOD_RETURN:

         DEBUG(SSSDBG_TRACE_FUNC, "DP Success\n");

@@ -146,10 +125,9 @@ static errno_t rdp_process_pending_call(DBusPendingCall *pending,

         DEBUG(SSSDBG_CRIT_FAILURE, "DP Error [%s]: %s\n",

               error.name, (error.message == NULL ? "(null)" : error.message));

-        ret = rdp_error_to_errno(&error);

+        ret = sbus_error_to_errno(&error);

         break;

     default:

-        dbus_message_unref(reply);

         DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected type?\n");

         ret = ERR_INTERNAL;

         goto done;

@@ -168,15 +146,6 @@ struct rdp_message_state {

     struct DBusMessage *reply;

 };

-static int rdp_message_state_destructor(struct rdp_message_state *state)

-{

-    if (state->reply != NULL) {

-        dbus_message_unref(state->reply);

-    }

-

-    return 0;

-}

-

 static void rdp_message_done(DBusPendingCall *pending, void *ptr);

 struct tevent_req *_rdp_message_send(TALLOC_CTX *mem_ctx,

@@ -199,8 +168,6 @@ struct tevent_req *_rdp_message_send(TALLOC_CTX *mem_ctx,

         return NULL;

     }

-    talloc_set_destructor(state, rdp_message_state_destructor);

-

     va_start(va, first_arg_type);

     ret = rdp_message_send_internal(rctx, domain, rdp_message_done, req,

                                     path, iface, method, first_arg_type, va);

@@ -233,14 +200,8 @@ static void rdp_message_done(DBusPendingCall *pending, void *ptr)

     req = talloc_get_type(ptr, struct tevent_req);

     state = tevent_req_data(req, struct rdp_message_state);

-    ret = rdp_process_pending_call(pending, &state->reply);

+    ret = rdp_process_pending_call(state, pending, &state->reply);

     if (ret != EOK) {

-        if (state->reply != NULL) {

-            dbus_message_unref(state->reply);

-        }

-

-        state->reply = NULL;

-

         tevent_req_error(req, ret);

         return;

     }

@@ -253,35 +214,17 @@ errno_t _rdp_message_recv(struct tevent_req *req,

                           ...)

 {

     struct rdp_message_state *state;

-    DBusError error;

-    dbus_bool_t bret;

     errno_t ret;

     va_list va;

     TEVENT_REQ_RETURN_ON_ERROR(req);

     state = tevent_req_data(req, struct rdp_message_state);

-    dbus_error_init(&error);

     va_start(va, first_arg_type);

-    bret = dbus_message_get_args_valist(state->reply, &error, first_arg_type, va);

+    ret = sbus_parse_message_valist(state->reply, false, first_arg_type, va);

     va_end(va);

-    if (bret == false) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to parse reply\n");

-        ret = EIO;

-        goto done;

-    }

-

-    ret = rdp_error_to_errno(&error);

-    if (ret != EOK) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to parse message [%s]: %s\n",

-              error.name, error.message);

-        goto done;

-    }

-

-done:

-    dbus_error_free(&error);

     return ret;

 }

@@ -317,7 +260,7 @@ static void rdp_message_send_and_reply_done(DBusPendingCall *pending,

                                             void *ptr)

 {

     struct sbus_request *sbus_req;

-    DBusMessage *reply = NULL;

+    DBusMessage *reply;

     dbus_uint32_t serial;

     const char *sender;

     dbus_bool_t dbret;

@@ -325,7 +268,7 @@ static void rdp_message_send_and_reply_done(DBusPendingCall *pending,

     sbus_req = talloc_get_type(ptr, struct sbus_request);

-    ret = rdp_process_pending_call(pending, &reply);

+    ret = rdp_process_pending_call(sbus_req, pending, &reply);

     if (reply == NULL) {

         /* Something bad happened. Just kill the request. */

         ret = EIO;

@@ -358,10 +301,6 @@ static void rdp_message_send_and_reply_done(DBusPendingCall *pending,

     ret = EOK;

 done:

-    if (reply != NULL) {

-        dbus_message_unref(reply);

-    }

-

     if (ret != EOK) {

         /* Something bad happend, just kill the request. */

         talloc_free(sbus_req);

diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h

index c0aedf36b496bfda05dcde921ea7060efb4cc91f..15e3b117e1a467f4e250cdf4ba8fd0326e4d380e 100644

--- a/src/sbus/sssd_dbus.h

+++ b/src/sbus/sssd_dbus.h

@@ -29,6 +29,8 @@ struct sbus_request;

 #include <dbus/dbus.h>

 #include <sys/types.h>

 #include "util/util.h"

+#include "sbus/sssd_dbus_errors.h"

+#include "sbus/sssd_dbus_utils.h"

 /* Older platforms (such as RHEL-6) might not have these error constants

  * defined */

diff --git a/src/sbus/sssd_dbus_utils.c b/src/sbus/sssd_dbus_utils.c

new file mode 100644

index 0000000000000000000000000000000000000000..4c33f9fd75cac2d4a56a5638982f8ecb73da8e2e

--- /dev/null

+++ b/src/sbus/sssd_dbus_utils.c

@@ -0,0 +1,226 @@

+/*

+    Authors:

+        Pavel Březina <pbrezina@redhat.com>

+

+    Copyright (C) 2016 Red Hat

+

+    This program is free software; you can redistribute it and/or modify

+    it under the terms of the GNU General Public License as published by

+    the Free Software Foundation; either version 3 of the License, or

+    (at your option) any later version.

+

+    This program is distributed in the hope that it will be useful,

+    but WITHOUT ANY WARRANTY; without even the implied warranty of

+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

+    GNU General Public License for more details.

+

+    You should have received a copy of the GNU General Public License

+    along with this program.  If not, see <http://www.gnu.org/licenses/>.

+*/

+

+#include <talloc.h>

+

+#include "sbus/sssd_dbus.h"

+#include "util/util.h"

+

+struct sbus_talloc_msg {

+    DBusMessage *msg;

+};

+

+static int sbus_talloc_msg_destructor(struct sbus_talloc_msg *talloc_msg)

+{

+    if (talloc_msg->msg == NULL) {

+        return 0;

+    }

+

+    dbus_message_unref(talloc_msg->msg);

+    return 0;

+}

+

+errno_t sbus_talloc_bound_message(TALLOC_CTX *mem_ctx, DBusMessage *msg)

+{

+    struct sbus_talloc_msg *talloc_msg;

+

+    talloc_msg = talloc(mem_ctx, struct sbus_talloc_msg);

+    if (talloc_msg == NULL) {

+        DEBUG(SSSDBG_CRIT_FAILURE,

+              "Unable to bound D-Bus message with talloc context!\n");

+        return ENOMEM;

+    }

+

+    talloc_msg->msg = msg;

+

+    talloc_set_destructor(talloc_msg, sbus_talloc_msg_destructor);

+

+    return EOK;

+}

+

+errno_t sbus_error_to_errno(DBusError *error)

+{

+    static struct {

+        const char *name;

+        errno_t ret;

+    } list[] = { { SBUS_ERROR_INTERNAL, ERR_INTERNAL },

+                 { SBUS_ERROR_NOT_FOUND, ENOENT },

+                 { SBUS_ERROR_UNKNOWN_DOMAIN, ERR_DOMAIN_NOT_FOUND },

+                 { SBUS_ERROR_DP_FATAL, ERR_TERMINATED },

+                 { SBUS_ERROR_DP_OFFLINE, ERR_OFFLINE },

+                 { SBUS_ERROR_DP_NOTSUP, ENOTSUP },

+                 { NULL, ERR_INTERNAL } };

+    int i;

+

+    if (!dbus_error_is_set(error)) {

+        return EOK;

+    }

+

+    for (i = 0; list[i].name != NULL; i++) {

+        if (dbus_error_has_name(error, list[i].name)) {

+            return list[i].ret;

+        }

+    }

+

+    return EIO;

+}

+

+errno_t sbus_check_reply(DBusMessage *reply)

+{

+    dbus_bool_t bret;

+    DBusError error;

+    errno_t ret;

+

+    dbus_error_init(&error);

+

+    switch (dbus_message_get_type(reply)) {

+    case DBUS_MESSAGE_TYPE_METHOD_RETURN:

+        ret = EOK;

+        goto done;

+

+    case DBUS_MESSAGE_TYPE_ERROR:

+        bret = dbus_set_error_from_message(&error, reply);

+        if (bret == false) {

+            DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read error from message\n");

+            ret = EIO;

+            goto done;

+        }

+

+        DEBUG(SSSDBG_CRIT_FAILURE, "D-Bus error [%s]: %s\n",

+              error.name, (error.message == NULL ? "(null)" : error.message));

+        ret = sbus_error_to_errno(&error);

+        goto done;

+    default:

+        DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected D-Bus message type?\n");

+        ret = ERR_INTERNAL;

+        goto done;

+    }

+

+done:

+    dbus_error_free(&error);

+

+    return ret;

+}

+

+DBusMessage *sbus_create_message_valist(TALLOC_CTX *mem_ctx,

+                                        const char *bus,

+                                        const char *path,

+                                        const char *iface,

+                                        const char *method,

+                                        int first_arg_type,

+                                        va_list va)

+{

+    DBusMessage *msg;

+    dbus_bool_t bret;

+    errno_t ret;

+

+    msg = dbus_message_new_method_call(bus, path, iface, method);

+    if (msg == NULL) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create message\n");

+        return NULL;

+    }

+

+    bret = dbus_message_append_args_valist(msg, first_arg_type, va);

+    if (!bret) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "Failed to build message\n");

+        ret = EIO;

+        goto done;

+    }

+

+    ret = sbus_talloc_bound_message(mem_ctx, msg);

+

+done:

+    if (ret != EOK) {

+        dbus_message_unref(msg);

+    }

+

+    return msg;

+}

+

+DBusMessage *_sbus_create_message(TALLOC_CTX *mem_ctx,

+                                  const char *bus,

+                                  const char *path,

+                                  const char *iface,

+                                  const char *method,

+                                  int first_arg_type,

+                                  ...)

+{

+    DBusMessage *msg;

+    va_list va;

+

+    va_start(va, first_arg_type);

+    msg = sbus_create_message_valist(mem_ctx, bus, path, iface, method,

+                                     first_arg_type, va);

+    va_end(va);

+

+    return msg;

+}

+

+errno_t sbus_parse_message_valist(DBusMessage *msg,

+                                  bool check_reply,

+                                  int first_arg_type,

+                                  va_list va)

+{

+    DBusError error;

+    dbus_bool_t bret;

+    errno_t ret;

+

+    if (check_reply) {

+        ret = sbus_check_reply(msg);

+        if (ret != EOK) {

+            return ret;

+        }

+    }

+

+    dbus_error_init(&error);

+

+    bret = dbus_message_get_args_valist(msg, &error, first_arg_type, va);

+    if (bret == false) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to parse D-Bus message\n");

+        ret = EIO;

+        goto done;

+    }

+

+    ret = sbus_error_to_errno(&error);

+    if (ret != EOK) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to parse D-Bus message [%s]: %s\n",

+              error.name, error.message);

+        goto done;

+    }

+

+done:

+    dbus_error_free(&error);

+    return ret;

+}

+

+errno_t _sbus_parse_message(DBusMessage *msg,

+                            bool check_reply,

+                            int first_arg_type,

+                            ...)

+{

+    errno_t ret;

+    va_list va;

+

+    va_start(va, first_arg_type);

+    ret = sbus_parse_message_valist(msg, check_reply, first_arg_type, va);

+    va_end(va);

+

+    return ret;

+}

diff --git a/src/sbus/sssd_dbus_utils.h b/src/sbus/sssd_dbus_utils.h

new file mode 100644

index 0000000000000000000000000000000000000000..74c21fb7930c7f5f5417b6a2587cf691b1bc0b19

--- /dev/null

+++ b/src/sbus/sssd_dbus_utils.h

@@ -0,0 +1,64 @@

+/*

+    Authors:

+        Pavel Březina <pbrezina@redhat.com>

+

+    Copyright (C) 2016 Red Hat

+

+    This program is free software; you can redistribute it and/or modify

+    it under the terms of the GNU General Public License as published by

+    the Free Software Foundation; either version 3 of the License, or

+    (at your option) any later version.

+

+    This program is distributed in the hope that it will be useful,

+    but WITHOUT ANY WARRANTY; without even the implied warranty of

+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

+    GNU General Public License for more details.

+

+    You should have received a copy of the GNU General Public License

+    along with this program.  If not, see <http://www.gnu.org/licenses/>.

+*/

+

+#ifndef SSSD_DBUS_UTILS_H_

+#define SSSD_DBUS_UTILS_H_

+

+errno_t sbus_talloc_bound_message(TALLOC_CTX *mem_ctx, DBusMessage *msg);

+errno_t sbus_error_to_errno(DBusError *error);

+errno_t sbus_check_reply(DBusMessage *reply);

+

+DBusMessage *sbus_create_message_valist(TALLOC_CTX *mem_ctx,

+                                        const char *bus,

+                                        const char *path,

+                                        const char *iface,

+                                        const char *method,

+                                        int first_arg_type,

+                                        va_list va);

+

+DBusMessage *_sbus_create_message(TALLOC_CTX *mem_ctx,

+                                  const char *bus,

+                                  const char *path,

+                                  const char *iface,

+                                  const char *method,

+                                  int first_arg_type,

+                                  ...);

+

+#define sbus_create_message(mem_ctx, bus, path, iface, method, ...) \

+    _sbus_create_message(mem_ctx, bus, path, iface, method,         \

+                         ##__VA_ARGS__, DBUS_TYPE_INVALID)

+

+errno_t sbus_parse_message_valist(DBusMessage *msg,

+                                  bool check_reply,

+                                  int first_arg_type,

+                                  va_list va);

+

+errno_t _sbus_parse_message(DBusMessage *msg,

+                            bool check_reply,

+                            int first_arg_type,

+                            ...);

+

+#define sbus_parse_message(msg, ...) \

+    _sbus_parse_message(msg, false, ##__VA_ARGS__, DBUS_TYPE_INVALID)

+

+#define sbus_parse_reply(msg, ...) \

+    _sbus_parse_message(msg, true, ##__VA_ARGS__, DBUS_TYPE_INVALID)

+

+#endif /* SSSD_DBUS_UTILS_H_ */

diff --git a/src/tools/sssctl/sssctl_domains.c b/src/tools/sssctl/sssctl_domains.c

index cfc4e56133213e27496350033d4d28c3f5b5c63d..17ad670f39dfc045ba090210ffcfa77df713c306 100644

--- a/src/tools/sssctl/sssctl_domains.c

+++ b/src/tools/sssctl/sssctl_domains.c

@@ -79,15 +79,11 @@ static errno_t sssctl_domain_status_online(struct sss_tool_ctx *tool_ctx,

 {

     sss_sifp_ctx *sifp;

     sss_sifp_error sifp_error;

-    DBusError dbus_error;

     DBusMessage *reply = NULL;

-    DBusMessage *msg = NULL;

+    DBusMessage *msg;

     bool is_online;

-    dbus_bool_t dbret;

     errno_t ret;

-    dbus_error_init(&dbus_error);

-

     if (!sssctl_start_sssd(force_start)) {

         ret = ERR_SSSD_NOT_RUNNING;

         goto done;

@@ -100,16 +96,15 @@ static errno_t sssctl_domain_status_online(struct sss_tool_ctx *tool_ctx,

         goto done;

     }

-

-    msg = sss_sifp_create_message(domain_path, IFACE_IFP_DOMAINS_DOMAIN,

-                                  IFACE_IFP_DOMAINS_DOMAIN_ISONLINE);

+    msg = sbus_create_message(tool_ctx, SSS_SIFP_ADDRESS, domain_path,

+                              IFACE_IFP_DOMAINS_DOMAIN,

+                              IFACE_IFP_DOMAINS_DOMAIN_ISONLINE);

     if (msg == NULL) {

         DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create D-Bus message\n");

         ret = ENOMEM;

         goto done;

     }

-

     sifp_error = sss_sifp_send_message(sifp, msg, &reply);

     if (sifp_error != SSS_SIFP_OK) {

         sssctl_sifp_error(sifp, sifp_error, "Unable to get online status");

@@ -117,16 +112,9 @@ static errno_t sssctl_domain_status_online(struct sss_tool_ctx *tool_ctx,

         goto done;

     }

-    dbret = dbus_message_get_args(reply, &dbus_error,

-                                  DBUS_TYPE_BOOLEAN, &is_online,

-                                  DBUS_TYPE_INVALID);

-    if (!dbret) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to parse D-Bus reply\n");

-        if (dbus_error_is_set(&dbus_error)) {

-            DEBUG(SSSDBG_CRIT_FAILURE, "%s: %s\n",

-                  dbus_error.name, dbus_error.message);

-        }

-        ret = EIO;

+    ret = sbus_parse_reply(reply, DBUS_TYPE_BOOLEAN, &is_online);

+    if (ret != EOK) {

+        fprintf(stderr, _("Unable to get information from SSSD\n"));

         goto done;

     }

@@ -135,16 +123,10 @@ static errno_t sssctl_domain_status_online(struct sss_tool_ctx *tool_ctx,

     ret = EOK;

 done:

-    if (msg != NULL) {

-        dbus_message_unref(msg);

-    }

-

     if (reply != NULL) {

         dbus_message_unref(reply);

     }

-    dbus_error_free(&dbus_error);

-

     return ret;

 }

-- 

2.4.11