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