From 439e08cdc5c83b3e5835cb0435983f1da2ffbaf1 Mon Sep 17 00:00:00 2001 From: Pavel Březina Date: Aug 16 2016 12:39:04 +0000 Subject: 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. Reviewed-by: Lukáš Slebodník Reviewed-by: Jakub Hrozek --- diff --git a/Makefile.am b/Makefile.am index 5d1d671..ebb6884 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 e226401..6ad2ba0 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 c0aedf3..15e3b11 100644 --- a/src/sbus/sssd_dbus.h +++ b/src/sbus/sssd_dbus.h @@ -29,6 +29,8 @@ struct sbus_request; #include #include #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 0000000..4c33f9f --- /dev/null +++ b/src/sbus/sssd_dbus_utils.c @@ -0,0 +1,226 @@ +/* + Authors: + Pavel Březina + + 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 . +*/ + +#include + +#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 0000000..74c21fb --- /dev/null +++ b/src/sbus/sssd_dbus_utils.h @@ -0,0 +1,64 @@ +/* + Authors: + Pavel Březina + + 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 . +*/ + +#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 cfc4e56..17ad670 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; }