From 2ad0b97f9e435ac5f57caf37479304c16d0424c9 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 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                           |  5 ++
 src/sbus/sssd_dbus_request.c                   | 73 ++++++++++++++++++++------
 4 files changed, 68 insertions(+), 33 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..3bce46b192a18d1e4c732e7fd3488ae17b84d10c 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,
+                              ...);
+
 /*
  * Construct a new DBusError instance which can be consumed by functions such
  * as @sbus_request_fail_and_finish().
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

