From 92eb609c7085628eb7a41715f06f2e7884029f7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Tue, 28 Jun 2016 15:29:39 +0200 Subject: [PATCH 2/7] sbus: add sbus_request_reply_error() This simplifies error handling in sbus requests since we avoid creating DBusError and checking for NULL manually. It removes few lines of code. This patch does not replace all calls to sbus_request_fail_and_finish since sometimes it is desirable to create the error manualy. But it replaces it in most recent places. --- src/providers/data_provider/dp_iface_backend.c | 11 ++-- src/responder/ifp/ifp_domains.c | 12 ++--- src/sbus/sssd_dbus.h | 7 ++- src/sbus/sssd_dbus_request.c | 73 ++++++++++++++++++++------ 4 files changed, 69 insertions(+), 34 deletions(-) diff --git a/src/providers/data_provider/dp_iface_backend.c b/src/providers/data_provider/dp_iface_backend.c index f4af35ed6ec3858b7fff80cf2933926a653ba6f5..d9a84bfee4c5c11e46e0e8f7021f829825ad95c1 100644 --- a/src/providers/data_provider/dp_iface_backend.c +++ b/src/providers/data_provider/dp_iface_backend.c @@ -34,7 +34,6 @@ errno_t dp_backend_is_online(struct sbus_request *sbus_req, { struct be_ctx *be_ctx; struct sss_domain_info *domain; - DBusError *error; bool online; be_ctx = dp_client_be(dp_cli); @@ -44,13 +43,9 @@ errno_t dp_backend_is_online(struct sbus_request *sbus_req, } else { domain = find_domain_by_name(be_ctx->domain, domname, false); if (domain == NULL) { - error = sbus_error_new(sbus_req, SBUS_ERROR_UNKNOWN_DOMAIN, - "Unknown domain %s", domname); - if (error == NULL) { - return ENOMEM; - } - - return sbus_request_fail_and_finish(sbus_req, error); + sbus_request_reply_error(sbus_req, SBUS_ERROR_UNKNOWN_DOMAIN, + "Unknown domain %s", domname); + return EOK; } } diff --git a/src/responder/ifp/ifp_domains.c b/src/responder/ifp/ifp_domains.c index 80c57a486d277b2bff21023f72be2ccffc72ba09..402f2f086bc6bb60f7736e685e124694cebc14ca 100644 --- a/src/responder/ifp/ifp_domains.c +++ b/src/responder/ifp/ifp_domains.c @@ -543,15 +543,13 @@ int ifp_domains_domain_is_online(struct sbus_request *sbus_req, { struct ifp_ctx *ifp_ctx; struct sss_domain_info *dom; - DBusError *error; ifp_ctx = talloc_get_type(data, struct ifp_ctx); dom = get_domain_info_from_req(sbus_req, data); if (dom == NULL) { - error = sbus_error_new(sbus_req, SBUS_ERROR_UNKNOWN_DOMAIN, - "Unknown domain"); - sbus_request_fail_and_finish(sbus_req, error); + sbus_request_reply_error(sbus_req, SBUS_ERROR_UNKNOWN_DOMAIN, + "Unknown domain"); return EOK; } @@ -567,15 +565,13 @@ int ifp_domains_domain_list_services(struct sbus_request *sbus_req, { struct ifp_ctx *ifp_ctx; struct sss_domain_info *dom; - DBusError *error; ifp_ctx = talloc_get_type(data, struct ifp_ctx); dom = get_domain_info_from_req(sbus_req, data); if (dom == NULL) { - error = sbus_error_new(sbus_req, SBUS_ERROR_UNKNOWN_DOMAIN, - "Unknown domain"); - sbus_request_fail_and_finish(sbus_req, error); + sbus_request_reply_error(sbus_req, SBUS_ERROR_UNKNOWN_DOMAIN, + "Unknown domain"); return EOK; } diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h index fe1c4a7e1730e088647744d9b49a68c3c71db57f..2fc6593b1131ee58ed044f526119298bbfd53104 100644 --- a/src/sbus/sssd_dbus.h +++ b/src/sbus/sssd_dbus.h @@ -357,6 +357,11 @@ int sbus_request_return_and_finish(struct sbus_request *dbus_req, int sbus_request_fail_and_finish(struct sbus_request *dbus_req, const DBusError *error); +void sbus_request_reply_error(struct sbus_request *sbus_req, + const char *error_name, + const char *fmt, + ...) SSS_ATTRIBUTE_PRINTF(3,4); + /* * Construct a new DBusError instance which can be consumed by functions such * as @sbus_request_fail_and_finish(). @@ -368,7 +373,7 @@ int sbus_request_fail_and_finish(struct sbus_request *dbus_req, * is duplicated using the returned DBusError instance as a talloc parent. */ DBusError *sbus_error_new(TALLOC_CTX *mem_ctx, - const char *dbus_err_name, + const char *dbus_error_name, const char *fmt, ...) SSS_ATTRIBUTE_PRINTF(3,4); diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c index f8647b5ecfb4a49d45a15733b22c6014f4bd084c..0cc9bb3db5bfbfb2eaad1fca76d6683d3ed0d0b1 100644 --- a/src/sbus/sssd_dbus_request.c +++ b/src/sbus/sssd_dbus_request.c @@ -199,31 +199,70 @@ int sbus_request_fail_and_finish(struct sbus_request *dbus_req, return ret; } +static DBusError *sbus_error_new_va(TALLOC_CTX *mem_ctx, + const char *error_name, + const char *fmt, + va_list ap) +{ + DBusError *error; + const char *error_msg; + + error = talloc_zero(mem_ctx, DBusError); + if (error == NULL) { + return NULL; + } + + if (fmt != NULL) { + error_msg = talloc_vasprintf(error, fmt, ap); + if (error_msg == NULL) { + talloc_free(error); + return NULL; + } + } else { + error_msg = NULL; + } + + dbus_error_init(error); + dbus_set_error_const(error, error_name, error_msg); + + return error; +} + DBusError *sbus_error_new(TALLOC_CTX *mem_ctx, - const char *dbus_err_name, + const char *dbus_error_name, const char *fmt, ...) { - DBusError *dberr; - const char *err_msg_dup = NULL; + DBusError *error; va_list ap; - dberr = talloc(mem_ctx, DBusError); - if (dberr == NULL) return NULL; - - if (fmt) { - va_start(ap, fmt); - err_msg_dup = talloc_vasprintf(dberr, fmt, ap); - va_end(ap); - if (err_msg_dup == NULL) { - talloc_free(dberr); - return NULL; - } + va_start(ap, fmt); + error = sbus_error_new_va(mem_ctx, dbus_error_name, fmt, ap); + va_end(ap); + + return error; +} + +void sbus_request_reply_error(struct sbus_request *sbus_req, + const char *error_name, + const char *fmt, + ...) +{ + DBusError *error; + va_list ap; + + va_start(ap, fmt); + error = sbus_error_new_va(sbus_req, error_name, fmt, ap); + va_end(ap); + + if (error == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create D-Bus error, " + "killing request!\n"); + talloc_free(sbus_req); + return; } - dbus_error_init(dberr); - dbus_set_error_const(dberr, dbus_err_name, err_msg_dup); - return dberr; + sbus_request_fail_and_finish(sbus_req, error); } struct array_arg { -- 2.1.0