From 9b0cda6876a5407b152bdeb51bb312aa52916172 Mon Sep 17 00:00:00 2001

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

Date: Tue, 28 Jun 2016 15:29:39 +0200

Subject: [PATCH 092/102] sbus: add sbus_request_reply_error()

MIME-Version: 1.0

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

Content-Transfer-Encoding: 8bit

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.

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

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

(cherry picked from commit a06e23c0bcf0c8669a29b801876aca8aac422931)

---

 src/providers/data_provider/dp_iface_backend.c | 11 ++--

 src/responder/ifp/ifp_domains.c                | 12 ++---

 src/sbus/sssd_dbus.h                           |  9 +++-

 src/sbus/sssd_dbus_request.c                   | 73 ++++++++++++++++++++------

 4 files changed, 70 insertions(+), 35 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 8bfd39feb39822921ea703d8a89ac372e0ad5410..ff690ed6a7d5519979d242a4d5dadd08aff50347 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..c0aedf36b496bfda05dcde921ea7060efb4cc91f 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,9 +373,9 @@ 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);

+                          ...) SSS_ATTRIBUTE_PRINTF(3, 4);

 /*

  * Parse a DBus method call request.

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

index f8647b5ecfb4a49d45a15733b22c6014f4bd084c..c5b08539ff85b5427e41f6e03991b40a0a43a7e3 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.4.11