From 5a69d0d178f99e7ac5c8eea91148b77c698044a7 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 3/7] sbus: add utility function to simplify message and reply
 handling

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.
---
 Makefile.am                                      |   2 +
 src/responder/common/data_provider/rdp_message.c |  85 ++-------
 src/sbus/sssd_dbus.h                             |   2 +
 src/sbus/sssd_dbus_utils.c                       | 227 +++++++++++++++++++++++
 src/sbus/sssd_dbus_utils.h                       |  64 +++++++
 src/tools/sssctl/sssctl_domains.c                |  32 +---
 6 files changed, 314 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 706b60d6a065e0a983f5a1cfbc26a78331c67d58..720ba8287b19847f5dd06c9718bbaae8195d0d5f 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 \
@@ -914,6 +915,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 3bce46b192a18d1e4c732e7fd3488ae17b84d10c..87729954a3cdfe11d86df33953ccfd5a5c11092f 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..967f6a61d148aea5a3df3dcb81dc44afadd555fd
--- /dev/null
+++ b/src/sbus/sssd_dbus_utils.c
@@ -0,0 +1,227 @@
+/*
+    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;
+        break;
+
+    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);
+        break;
+    default:
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected D-Bus message type?\n");
+        ret = ERR_INTERNAL;
+        break;
+    }
+
+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 5aaaf770981a92b80b9376fd244d2d97e0502dac..e822640a1c0bee3d33219fdbd6f976ec2b60dfe3 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.1.0

