On 08/16/2016 12:55 PM, Lukas Slebodnik wrote:
On (16/08/16 11:32), Pavel Březina wrote:
On 08/15/2016 01:21 PM, Lukas Slebodnik wrote:
On (15/08/16 11:49), Pavel Březina wrote:
On 08/15/2016 11:21 AM, Lukas Slebodnik wrote:

+void sbus_request_reply_error(struct sbus_request *sbus_req,
+                              const char *error_name,
+                              const char *fmt,
+                              ...);
+
It would be good to check format string at compile time
; add SSS_ATTRIBUTE_PRINTF

Done.

DBusError *sbus_error_new(TALLOC_CTX *mem_ctx,
-                          const char *dbus_err_name,
+                          const char *dbus_error_name,
                             const char *fmt,
                             ...)
It would be good to change the name also in header file.

Done.

+    sbus_request_fail_and_finish(sbus_req, error);
Is there a special reason why return code of this function is ignored?
If yes then the line deserve a comment.

Because the return code is useless and the function should be void. Feel free
to file a ticket/patch.

  From bca6975c580cbc16957dbb72691dbd16e1b66e7b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Thu, 14 Jul 2016 10:49:37 +0200
Subject: [PATCH 7/7] sifp: fix coverity warning

sssd-1.14.1/src/lib/sifp/sss_sifp_dbus.c:51: check_return: Calling 
"dbus_message_append_args_valist" without checking return value (as is done 
elsewhere 4 out of 5 times).
---
src/lib/sifp/sss_sifp_dbus.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/lib/sifp/sss_sifp_dbus.c b/src/lib/sifp/sss_sifp_dbus.c
index 
7c72c52f0d226ccdfaf7b8ffaed7776647a7771c..2906c5ac383c412231127f6ffa8081d47eb2bced
 100644
--- a/src/lib/sifp/sss_sifp_dbus.c
+++ b/src/lib/sifp/sss_sifp_dbus.c
@@ -36,6 +36,7 @@ static sss_sifp_error sss_sifp_ifp_call(sss_sifp_ctx *ctx,
{
      DBusMessage *msg = NULL;
      sss_sifp_error ret;
+   dbus_bool_t bret;

      if (object_path == NULL || interface == NULL || method == NULL) {
          return SSS_SIFP_INVALID_ARGUMENT;
@@ -48,7 +49,11 @@ static sss_sifp_error sss_sifp_ifp_call(sss_sifp_ctx *ctx,
      }

      if (first_arg_type != DBUS_TYPE_INVALID) {
-       dbus_message_append_args_valist(msg, first_arg_type, ap);
+       bret = dbus_message_append_args_valist(msg, first_arg_type, ap);
+       if (!bret) {
+           ret = SSS_SIFP_IO_ERROR;
+           goto done;
+       }
      }

Is this coverity warning also in master?

If answer is no then It would be better to squash it to the patch which
introduced the warning.
Very likely
     sss_sifp: make it compatible with latest version of the infopipe

I don't know, feel free to squash it.


+    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


 From 67f02e1d32b346181c9ac971ea963c9af56aa674 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Mon, 27 Jun 2016 13:56:13 +0200
Subject: [PATCH 6/7] sssctl: print active server and server list

---
src/providers/data_provider/dp_iface.c           |   4 +-
src/providers/data_provider/dp_iface.h           |   8 +
src/providers/data_provider/dp_iface.xml         |   8 +
src/providers/data_provider/dp_iface_failover.c  | 286 ++++++++++++++++++++++-
src/providers/data_provider/dp_iface_generated.c |  52 +++++
src/providers/data_provider/dp_iface_generated.h |  10 +
src/providers/fail_over.c                        |  42 ++++
src/providers/fail_over.h                        |   4 +
src/responder/ifp/ifp_domains.c                  |  46 ++++
src/responder/ifp/ifp_domains.h                  |   8 +
src/responder/ifp/ifp_iface.c                    |   4 +-
src/responder/ifp/ifp_iface.xml                  |  10 +
src/responder/ifp/ifp_iface_generated.c          |  52 +++++
src/responder/ifp/ifp_iface_generated.h          |  10 +
src/tools/sssctl/sssctl_domains.c                | 182 ++++++++++++++-
15 files changed, 708 insertions(+), 18 deletions(-)

//snip
errno_t dp_failover_list_services(struct sbus_request *sbus_req,
                                    void *dp_cli,
                                    const char *domname)
{
+    struct sss_domain_info *domain;
      struct be_ctx *be_ctx;
      struct be_svc_data *svc;
      const char **services;
      int num_services;
+    errno_t ret;
+    bool ad_found = false;
+    bool ipa_found = false;

      be_ctx = dp_client_be(dp_cli);

+    if (SBUS_IS_STRING_EMPTY(domname)) {
+        domain = be_ctx->domain;
+    } else {
+        domain = find_domain_by_name(be_ctx->domain, domname, false);
+        if (domain == NULL) {
+            sbus_request_reply_error(sbus_req, SBUS_ERROR_UNKNOWN_DOMAIN,
+                                     "Unknown domain %s", domname);
+            return EOK;
+        }
+    }
+
+    /**
+     * Returning list of failover services is currently rather difficult
+     * since there is only one failover context for the whole backend.
+     *
+     * The list of services for the given domain depends on whether it is
+     * a master domain or a subdomain and whether we are using IPA, AD or
+     * LDAP backend.
+     *
+     * For LDAP we just return everything we have.
+     * For AD master domain we return AD, AD_GC.
+     * For AD subdomain we return subdomain.com, AD_GC.
+     * For IPA in client mode we return IPA.
+     * For IPA in server mode we return IPA for master domain and
+     * subdomain.com, gc_subdomain.com for subdomain.
+     *
+     * We also return everything else for all cases if any other service
+     * such as kerberos is configured separately.
+     */
+
+    /* Allocate enough space. */
      num_services = 0;
      DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
          num_services++;
+
+        if (strcasecmp(svc->name, "AD") == 0) {
+            ad_found = true;
+        } else if (strcasecmp(svc->name, "IPA") == 0) {
+            ipa_found = true;
+        }
      }

      services = talloc_zero_array(sbus_req, const char *, num_services);
@@ -50,17 +232,103 @@ errno_t dp_failover_list_services(struct sbus_request 
*sbus_req,
          return ENOMEM;
      }

-    num_services = 0;
-    DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
-        services[num_services] = talloc_strdup(services, svc->name);
-        if (services[num_services] == NULL) {
-            DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup() failed\n");
-            talloc_free(services);
-            return ENOMEM;
-        }
-        num_services++;
+    /* Fill the list. */
+    ret = ERR_INTERNAL;
+    if ((ad_found && ipa_found) || (!ad_found && !ipa_found)) {
+        /* If AD and IPA was found it is some complicated configuration,
+         * we return everything. Otherwise it's LDAP. */
+        ret = dp_failover_list_services_ldap(be_ctx, services, &num_services);
+    } else if (ad_found) {
+        ret = dp_failover_list_services_ad(be_ctx, domain,
+                                           services, &num_services);
+    } else if (ipa_found) {
+        ret = dp_failover_list_services_ipa(be_ctx, domain,
+                                            services, &num_services);
+    }
+
If no branch of the above match, ret is undefined (thanks, Coverity)
I think you might overlook Jakub's comment regarding this part of code.
and I do not think it's is just about coverity.

I did not overlooked it, I initialized ret prior to if's.


What about creating and bit-mask style enum
enum {
      SIMPLE_LDAP = 0,
      AD_FOUND = 1,
      IPA_FOUND = 1 << 1,
      COMPLICATED_SETUP = AD_FOUND | IPA_FOUND
}   ^^^^^^^^^^^^
      or (MIXED_IPA_AD, IPA_AD_TRUST) ,,,

I think it would make it simpler to "parse" the code for Coverity
and also for mortal humas.

Nice idea. See new patches.


From d09005dea74a0b9e5d18730d4a372b5a6f4d8180 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Mon, 27 Jun 2016 13:56:13 +0200
Subject: [PATCH 6/7] sssctl: print active server and server list

---
src/providers/data_provider/dp_iface.c           |   4 +-
src/providers/data_provider/dp_iface.h           |   8 +
src/providers/data_provider/dp_iface.xml         |   8 +
src/providers/data_provider/dp_iface_failover.c  | 294 ++++++++++++++++++++++-
src/providers/data_provider/dp_iface_generated.c |  52 ++++
src/providers/data_provider/dp_iface_generated.h |  10 +
src/providers/fail_over.c                        |  42 ++++
src/providers/fail_over.h                        |   4 +
src/responder/ifp/ifp_domains.c                  |  46 ++++
src/responder/ifp/ifp_domains.h                  |   8 +
src/responder/ifp/ifp_iface.c                    |   4 +-
src/responder/ifp/ifp_iface.xml                  |  10 +
src/responder/ifp/ifp_iface_generated.c          |  52 ++++
src/responder/ifp/ifp_iface_generated.h          |  10 +
src/tools/sssctl/sssctl_domains.c                | 182 +++++++++++++-
15 files changed, 716 insertions(+), 18 deletions(-)

diff --git a/src/providers/data_provider/dp_iface_failover.c 
b/src/providers/data_provider/dp_iface_failover.c
index 
038791088eeab7e9c5923996db77d2a107ff067d..5016b4afbb66943855114576813fc3e69fd230b9
 100644
--- a/src/providers/data_provider/dp_iface_failover.c
+++ b/src/providers/data_provider/dp_iface_failover.c
//snip
+static errno_t
+dp_failover_list_services_ad(struct be_ctx *be_ctx,
+                             struct sss_domain_info *domain,
+                             const char **services,
+                             int *_count)
+{
+    char *fo_svc_name = NULL;
+    struct be_svc_data *svc;
+    errno_t ret;
+    int count;
+
+    fo_svc_name = talloc_asprintf(services, "sd_%s", domain->name);
+    if (fo_svc_name == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    count = 0;
+    DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
+        /* Drop each sd_gc_* since this service is not used with AD at all,
+         * we only connect to AD_GC for global catalog. */
+        if (strncasecmp(svc->name, "sd_gc_", strlen("sd_gc_")) == 0) {
+            continue;
+        }
+
+        /* Drop all subdomain services for different domain. */
+        if (strncasecmp(svc->name, "sd_", strlen("sd_")) == 0) {
+            if (!IS_SUBDOMAIN(domain)) {
+                continue;
+            }
+
+            if (strcasecmp(svc->name, fo_svc_name) != 0) {
+                continue;
+            }
+        }
+
+        if (IS_SUBDOMAIN(domain)) {
+            /* Drop AD since we connect to subdomain.com for LDAP. */
+            if (strcasecmp(svc->name, "AD") == 0) {
+                continue;
+            }
+        }
+
+        services[count] = talloc_strdup(services, svc->name);
+        if (services[count] == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup() failed\n");
+            ret = ENOMEM;
+            goto done;
+        }
+        count++;
+    }
+
+    *_count = count;
+
+    ret = EOK;
+
+done:
+    talloc_free(fo_svc_name);
+    return ret;
+}
+
+static errno_t
+dp_failover_list_services_ipa(struct be_ctx *be_ctx,
+                              struct sss_domain_info *domain,
+                              const char **services,
+                              int *_count)
+{
+    struct be_svc_data *svc;
+    char *fo_svc_name = NULL;
+    char *fo_gc_name = NULL;
+    errno_t ret;
+    int count;
+
+    fo_svc_name = talloc_asprintf(services, "sd_%s", domain->name);
+    if (fo_svc_name == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    fo_gc_name = talloc_asprintf(services, "sd_gc_%s", domain->name);
+    if (fo_gc_name == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    count = 0;
+    DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
+        /* Drop all subdomain services for different domain. */
+        if (strncasecmp(svc->name, "sd_", strlen("sd_")) == 0) {
+            if (!IS_SUBDOMAIN(domain)) {
+                continue;
+            }
+
+            if (strcasecmp(svc->name, fo_svc_name) != 0
+                    && strcasecmp(svc->name, fo_gc_name) != 0) {
+                continue;
+            }
+        }
+
+        services[count] = talloc_strdup(services, svc->name);
+        if (services[count] == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup() failed\n");
+            return ENOMEM;
+        }
+        count++;
+    }
+
+    *_count = count;
+
+    ret = EOK;
+
+done:
+    talloc_free(fo_svc_name);
+    talloc_free(fo_gc_name);
+
+    return ret;
+}
+
+enum dp_fo_svc_type {
+    DP_FO_SVC_LDAP = 0,
+    DP_FO_SVC_AD = 1,
+    DP_FO_SVC_IPA = 1 << 1,
+    DP_FO_SVC_MIXED = DP_FO_SVC_AD | DP_FO_SVC_IPA
+};
+
errno_t dp_failover_list_services(struct sbus_request *sbus_req,
                                   void *dp_cli,
                                   const char *domname)
{
+    enum dp_fo_svc_type svc_type = DP_FO_SVC_LDAP;
+    struct sss_domain_info *domain;
     struct be_ctx *be_ctx;
     struct be_svc_data *svc;
     const char **services;
     int num_services;
+    errno_t ret;

     be_ctx = dp_client_be(dp_cli);

+    if (SBUS_IS_STRING_EMPTY(domname)) {
+        domain = be_ctx->domain;
+    } else {
+        domain = find_domain_by_name(be_ctx->domain, domname, false);
+        if (domain == NULL) {
+            sbus_request_reply_error(sbus_req, SBUS_ERROR_UNKNOWN_DOMAIN,
+                                     "Unknown domain %s", domname);
+            return EOK;
+        }
+    }
+
+    /**
+     * Returning list of failover services is currently rather difficult
+     * since there is only one failover context for the whole backend.
+     *
+     * The list of services for the given domain depends on whether it is
+     * a master domain or a subdomain and whether we are using IPA, AD or
+     * LDAP backend.
+     *
+     * For LDAP we just return everything we have.
+     * For AD master domain we return AD, AD_GC.
+     * For AD subdomain we return subdomain.com, AD_GC.
+     * For IPA in client mode we return IPA.
+     * For IPA in server mode we return IPA for master domain and
+     * subdomain.com, gc_subdomain.com for subdomain.
+     *
+     * We also return everything else for all cases if any other service
+     * such as kerberos is configured separately.
+     */
+
+    /* Allocate enough space. */
     num_services = 0;
     DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
         num_services++;
+
+        if (strcasecmp(svc->name, "AD") == 0) {
+            svc_type |= DP_FO_SVC_AD;
+        } else if (strcasecmp(svc->name, "IPA") == 0) {
+            svc_type |= DP_FO_SVC_IPA;
+        }
     }

     services = talloc_zero_array(sbus_req, const char *, num_services);
@@ -50,17 +238,105 @@ errno_t dp_failover_list_services(struct sbus_request 
*sbus_req,
         return ENOMEM;
     }

-    num_services = 0;
-    DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
-        services[num_services] = talloc_strdup(services, svc->name);
-        if (services[num_services] == NULL) {
-            DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup() failed\n");
-            talloc_free(services);
-            return ENOMEM;
-        }
-        num_services++;
+    /* Fill the list. */
+    switch (svc_type) {
+    case DP_FO_SVC_LDAP:
+    case DP_FO_SVC_MIXED:
+        ret = dp_failover_list_services_ldap(be_ctx, services, &num_services);
+        break;
+    case DP_FO_SVC_AD:
+        ret = dp_failover_list_services_ad(be_ctx, domain,
+                                           services, &num_services);
+        break;
+    case DP_FO_SVC_IPA:
+        ret = dp_failover_list_services_ipa(be_ctx, domain,
+                                            services, &num_services);
+        break;
+    }
+
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create service list [%d]: %s\n",
+              ret, sss_strerror(ret));
+        talloc_free(services);
+        return ret;
     }
coverity is happy with this version
but gcc-6.1 (at least) with -O2 does not like it.

src/providers/data_provider/dp_iface_failover.c: In function 
‘dp_failover_list_services’:
src/providers/data_provider/dp_iface_failover.c:257:8: error: ‘ret’ may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
      if (ret != EOK) {
         ^

Yes, it's false possitive but we do not have any warning
maybe-uninitialized in master. And AS you can see I compile with Werror :-)


Thanks. Fixed.

From ac977e4730b0efc68c43b5bc41f14c9f5c6f0990 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Tue, 28 Jun 2016 11:40:16 +0200
Subject: [PATCH 1/7] rdp: add ability to forward reply to the client request

In cases where the InfoPipe servers just as a middle-man between
the DataProvider and a client we can simply forward the reply
reducing amount of coded needed in the InfoPipe.
---
 src/responder/common/data_provider/rdp.h         |  20 ++
 src/responder/common/data_provider/rdp_message.c | 260 +++++++++++++++++------
 src/responder/ifp/ifp_domains.c                  |  73 +------
 3 files changed, 217 insertions(+), 136 deletions(-)

diff --git a/src/responder/common/data_provider/rdp.h b/src/responder/common/data_provider/rdp.h
index 8a3ec803d8ea7914e2a54661e31dec084586582a..f0aed179a3d33de0462591e6c0bcd3c518105707 100644
--- a/src/responder/common/data_provider/rdp.h
+++ b/src/responder/common/data_provider/rdp.h
@@ -54,6 +54,26 @@ errno_t _rdp_message_recv(struct tevent_req *req,
 #define rdp_message_recv(req, ...)                                         \
     _rdp_message_recv(req, ##__VA_ARGS__, DBUS_TYPE_INVALID)
 
+/**
+ * Send D-Bus message to Data Provider but instead of returning the reply
+ * to the caller it forwards the reply to the client request. No further
+ * processing is required by the caller. In case of a failure the client
+ * request is freed since there is nothing we can do.
+ */
+void _rdp_message_send_and_reply(struct sbus_request *sbus_req,
+                                 struct resp_ctx *rctx,
+                                 struct sss_domain_info *domain,
+                                 const char *path,
+                                 const char *iface,
+                                 const char *method,
+                                 int first_arg_type,
+                                 ...);
+
+#define rdp_message_send_and_reply(sbus_req, rctx, domain, path, iface,       \
+                                   method, ...)                               \
+    _rdp_message_send_and_reply(sbus_req, rctx, domain, path, iface, method,  \
+                                ##__VA_ARGS__, DBUS_TYPE_INVALID)
+
 errno_t rdp_register_client(struct be_conn *be_conn,
                             const char *client_name);
 
diff --git a/src/responder/common/data_provider/rdp_message.c b/src/responder/common/data_provider/rdp_message.c
index 78af6f8967b378536b6456274fbcac4b609d1033..e226401567e4a1b2b9784a9aba21540ff5f0bc8d 100644
--- a/src/responder/common/data_provider/rdp_message.c
+++ b/src/responder/common/data_provider/rdp_message.c
@@ -53,104 +53,72 @@ static errno_t rdp_error_to_errno(DBusError *error)
     return EIO;
 }
 
-struct rdp_message_state {
-    struct DBusMessage *reply;
-};
-
-static int rdp_message_state_destructor(struct rdp_message_state *state)
+static errno_t
+rdp_message_send_internal(struct resp_ctx *rctx,
+                          struct sss_domain_info *domain,
+                          DBusPendingCallNotifyFunction notify_fn,
+                          void *notify_fn_data,
+                          const char *path,
+                          const char *iface,
+                          const char *method,
+                          int first_arg_type,
+                          va_list va)
 {
-    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,
-                                     struct resp_ctx *rctx,
-                                     struct sss_domain_info *domain,
-                                     const char *path,
-                                     const char *iface,
-                                     const char *method,
-                                     int first_arg_type,
-                                     ...)
-{
-    struct rdp_message_state *state;
     struct be_conn *be_conn;
-    struct tevent_req *req;
-    DBusMessage *msg;
+    DBusMessage *msg = NULL;
     dbus_bool_t bret;
     errno_t ret;
-    va_list va;
-
-    req = tevent_req_create(mem_ctx, &state, struct rdp_message_state);
-    if (req == NULL) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "tevent_req_create() failed\n");
-        return NULL;
-    }
-
-    talloc_set_destructor(state, rdp_message_state_destructor);
 
     ret = sss_dp_get_domain_conn(rctx, domain->conn_name, &be_conn);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "BUG: The Data Provider connection for "
               "%s is not available!\n", domain->name);
-        ret = ERR_INTERNAL;
-        goto immediately;
+        goto done;
     }
 
     msg = dbus_message_new_method_call(NULL, path, iface, method);
     if (msg == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create message\n");
         ret = ENOMEM;
-        goto immediately;
+        goto done;
     }
 
-    va_start(va, first_arg_type);
     bret = dbus_message_append_args_valist(msg, first_arg_type, va);
-    va_end(va);
     if (!bret) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Failed to build message\n");
         ret = EIO;
-        goto immediately;
+        goto done;
     }
 
     DEBUG(SSSDBG_TRACE_FUNC, "DP Request: %s %s.%s\n", path, iface, method);
 
-    ret = sbus_conn_send(be_conn->conn, msg, 30000,
-                         rdp_message_done, req, NULL);
+    ret = sbus_conn_send(be_conn->conn, msg, 3000,
+                         notify_fn, notify_fn_data, NULL);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Unable to contact Data Provider "
               "[%d]: %s\n", ret, sss_strerror(ret));
-        goto immediately;
+        goto done;
     }
 
-    return req;
+    ret = EOK;
 
-immediately:
-    if (ret == EOK) {
-        tevent_req_done(req);
-    } else {
-        tevent_req_error(req, ret);
+done:
+    if (msg != NULL) {
+        dbus_message_unref(msg);
     }
-    tevent_req_post(req, rctx->ev);
 
-    return req;
+    return ret;
 }
 
-static void rdp_message_done(DBusPendingCall *pending, void *ptr)
+static errno_t rdp_process_pending_call(DBusPendingCall *pending,
+                                        DBusMessage **_reply)
 {
-    struct rdp_message_state *state;
-    DBusMessage *reply = NULL;
-    struct tevent_req *req;
-    DBusError error;
+    DBusMessage *reply;
     dbus_bool_t bret;
+    DBusError error;
     errno_t ret;
 
-    req = talloc_get_type(ptr, struct tevent_req);
-    state = tevent_req_data(req, struct rdp_message_state);
+    *_reply = NULL;
 
     dbus_error_init(&error);
 
@@ -165,9 +133,8 @@ static void rdp_message_done(DBusPendingCall *pending, void *ptr)
     switch (dbus_message_get_type(reply)) {
     case DBUS_MESSAGE_TYPE_METHOD_RETURN:
         DEBUG(SSSDBG_TRACE_FUNC, "DP Success\n");
-        state->reply = reply;
         ret = EOK;
-        goto done;
+        break;
 
     case DBUS_MESSAGE_TYPE_ERROR:
         bret = dbus_set_error_from_message(&error, reply);
@@ -180,28 +147,105 @@ static void rdp_message_done(DBusPendingCall *pending, void *ptr)
         DEBUG(SSSDBG_CRIT_FAILURE, "DP Error [%s]: %s\n",
               error.name, (error.message == NULL ? "(null)" : error.message));
         ret = rdp_error_to_errno(&error);
-        goto done;
+        break;
     default:
+        dbus_message_unref(reply);
         DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected type?\n");
         ret = ERR_INTERNAL;
         goto done;
     }
 
-    ret = ERR_INTERNAL;
+    *_reply = reply;
 
 done:
     dbus_pending_call_unref(pending);
     dbus_error_free(&error);
 
+    return ret;
+}
+
+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,
+                                     struct resp_ctx *rctx,
+                                     struct sss_domain_info *domain,
+                                     const char *path,
+                                     const char *iface,
+                                     const char *method,
+                                     int first_arg_type,
+                                     ...)
+{
+    struct rdp_message_state *state;
+    struct tevent_req *req;
+    errno_t ret;
+    va_list va;
+
+    req = tevent_req_create(mem_ctx, &state, struct rdp_message_state);
+    if (req == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "tevent_req_create() failed\n");
+        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);
+    va_end(va);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to contact Data Provider "
+              "[%d]: %s\n", ret, sss_strerror(ret));
+        goto immediately;
+    }
+
+    return req;
+
+immediately:
     if (ret == EOK) {
         tevent_req_done(req);
+    } else {
+        tevent_req_error(req, ret);
+    }
+    tevent_req_post(req, rctx->ev);
+
+    return req;
+}
+
+static void rdp_message_done(DBusPendingCall *pending, void *ptr)
+{
+    struct rdp_message_state *state;
+    struct tevent_req *req;
+    errno_t ret;
+
+    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);
+    if (ret != EOK) {
+        if (state->reply != NULL) {
+            dbus_message_unref(state->reply);
+        }
+
+        state->reply = NULL;
+
+        tevent_req_error(req, ret);
         return;
     }
 
-    if (reply != NULL) {
-        dbus_message_unref(reply);
-    }
-    tevent_req_error(req, ret);
+    tevent_req_done(req);
 }
 
 errno_t _rdp_message_recv(struct tevent_req *req,
@@ -241,3 +285,85 @@ done:
     return ret;
 }
 
+static void rdp_message_send_and_reply_done(DBusPendingCall *pending,
+                                            void *ptr);
+
+void _rdp_message_send_and_reply(struct sbus_request *sbus_req,
+                                 struct resp_ctx *rctx,
+                                 struct sss_domain_info *domain,
+                                 const char *path,
+                                 const char *iface,
+                                 const char *method,
+                                 int first_arg_type,
+                                 ...)
+{
+    errno_t ret;
+    va_list va;
+
+    va_start(va, first_arg_type);
+    ret = rdp_message_send_internal(rctx, domain,
+                                    rdp_message_send_and_reply_done, sbus_req,
+                                    path, iface, method, first_arg_type, va);
+    va_end(va);
+
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to contact Data Provider "
+              "[%d]: %s\n", ret, sss_strerror(ret));
+        talloc_free(sbus_req);
+    }
+}
+
+static void rdp_message_send_and_reply_done(DBusPendingCall *pending,
+                                            void *ptr)
+{
+    struct sbus_request *sbus_req;
+    DBusMessage *reply = NULL;
+    dbus_uint32_t serial;
+    const char *sender;
+    dbus_bool_t dbret;
+    errno_t ret;
+
+    sbus_req = talloc_get_type(ptr, struct sbus_request);
+
+    ret = rdp_process_pending_call(pending, &reply);
+    if (reply == NULL) {
+        /* Something bad happened. Just kill the request. */
+        ret = EIO;
+        goto done;
+    }
+
+    /* Otherwise we have a valid reply and we do not care about returned
+     * value. We set destination and serial in reply to point to the original
+     * client request. */
+
+    sender = dbus_message_get_sender(sbus_req->message);
+    serial = dbus_message_get_serial(sbus_req->message);
+
+    dbret = dbus_message_set_destination(reply, sender);
+    if (dbret == false) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to set reply sender!\n");
+        ret = EIO;
+        goto done;
+    }
+
+    dbret = dbus_message_set_reply_serial(reply, serial);
+    if (dbret == false) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to set reply serial!\n");
+        ret = EIO;
+        goto done;
+    }
+
+    sbus_request_finish(sbus_req, reply);
+
+    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/responder/ifp/ifp_domains.c b/src/responder/ifp/ifp_domains.c
index 5333b25275e0847015f9cdd294eab5dbdda32f6c..80c57a486d277b2bff21023f72be2ccffc72ba09 100644
--- a/src/responder/ifp/ifp_domains.c
+++ b/src/responder/ifp/ifp_domains.c
@@ -538,14 +538,11 @@ void ifp_dom_get_parent_domain(struct sbus_request *dbus_req,
                                dom->parent->name);
 }
 
-static void ifp_domains_domain_is_online_done(struct tevent_req *req);
-
 int ifp_domains_domain_is_online(struct sbus_request *sbus_req,
                                  void *data)
 {
     struct ifp_ctx *ifp_ctx;
     struct sss_domain_info *dom;
-    struct tevent_req *req;
     DBusError *error;
 
     ifp_ctx = talloc_get_type(data, struct ifp_ctx);
@@ -558,49 +555,18 @@ int ifp_domains_domain_is_online(struct sbus_request *sbus_req,
         return EOK;
     }
 
-    req = rdp_message_send(sbus_req, ifp_ctx->rctx, dom, DP_PATH,
-                           IFACE_DP_BACKEND, IFACE_DP_BACKEND_ISONLINE,
-                           DBUS_TYPE_STRING, &dom->name);
-    if (req == NULL) {
-        return ENOMEM;
-    }
-
-    tevent_req_set_callback(req, ifp_domains_domain_is_online_done, sbus_req);
+    rdp_message_send_and_reply(sbus_req, ifp_ctx->rctx, dom, DP_PATH,
+                               IFACE_DP_BACKEND, IFACE_DP_BACKEND_ISONLINE,
+                               DBUS_TYPE_STRING, &dom->name);
 
     return EOK;
 }
 
-static void ifp_domains_domain_is_online_done(struct tevent_req *req)
-{
-    struct sbus_request *sbus_req;
-    DBusError *error;
-    bool is_online;
-    errno_t ret;
-
-    sbus_req = tevent_req_callback_data(req, struct sbus_request);
-
-    ret = rdp_message_recv(req, DBUS_TYPE_BOOLEAN, &is_online);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, "Unable to get online status [%d]: %s\n",
-              ret, sss_strerror(ret));
-        error = sbus_error_new(sbus_req, DBUS_ERROR_FAILED,
-                               "Unable to get online status [%d]: %s",
-                               ret, sss_strerror(ret));
-        sbus_request_fail_and_finish(sbus_req, error);
-        return;
-    }
-
-    iface_ifp_domains_domain_IsOnline_finish(sbus_req, is_online);
-}
-
-static void ifp_domains_domain_list_services_done(struct tevent_req *req);
-
 int ifp_domains_domain_list_services(struct sbus_request *sbus_req,
                                      void *data)
 {
     struct ifp_ctx *ifp_ctx;
     struct sss_domain_info *dom;
-    struct tevent_req *req;
     DBusError *error;
 
     ifp_ctx = talloc_get_type(data, struct ifp_ctx);
@@ -613,40 +579,9 @@ int ifp_domains_domain_list_services(struct sbus_request *sbus_req,
         return EOK;
     }
 
-    req = rdp_message_send(sbus_req, ifp_ctx->rctx, dom, DP_PATH,
+    rdp_message_send_and_reply(sbus_req, ifp_ctx->rctx, dom, DP_PATH,
                            IFACE_DP_FAILOVER, IFACE_DP_FAILOVER_LISTSERVICES,
                            DBUS_TYPE_STRING, &dom->name);
-    if (req == NULL) {
-        return ENOMEM;
-    }
-
-    tevent_req_set_callback(req, ifp_domains_domain_list_services_done, sbus_req);
 
     return EOK;
 }
-
-static void ifp_domains_domain_list_services_done(struct tevent_req *req)
-{
-    struct sbus_request *sbus_req;
-    DBusError *error;
-    int num_services;
-    const char **services;
-    errno_t ret;
-
-    sbus_req = tevent_req_callback_data(req, struct sbus_request);
-
-    ret = rdp_message_recv(req, DBUS_TYPE_ARRAY, DBUS_TYPE_STRING,
-                           &services, &num_services);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, "Unable to get failover services [%d]: %s\n",
-              ret, sss_strerror(ret));
-        error = sbus_error_new(sbus_req, DBUS_ERROR_FAILED,
-                               "Unable to get failover services [%d]: %s",
-                               ret, sss_strerror(ret));
-        sbus_request_fail_and_finish(sbus_req, error);
-        return;
-    }
-
-    iface_ifp_domains_domain_ListServices_finish(sbus_req, services,
-                                                 num_services);
-}
-- 
2.1.0

From 92eb609c7085628eb7a41715f06f2e7884029f7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@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                           |  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

From 75208ee43394e47f8d67dee07a1c9d70e5388279 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Wed, 29 Jun 2016 12:35:59 +0200
Subject: [PATCH 3/7] 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.
---
 Makefile.am                                      |   2 +
 src/responder/common/data_provider/rdp_message.c |  85 ++-------
 src/sbus/sssd_dbus.h                             |   2 +
 src/sbus/sssd_dbus_utils.c                       | 227 +++++++++++++++++++++++
 src/sbus/sssd_dbus_utils.h                       |  64 +++++++
 src/tools/sssctl/sssctl_domains.c                |  32 +---
 6 files changed, 314 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 5d1d671096f986d9387e6199112c017e9bf30e1b..ebb68848f578e9daf990c7659e463632d63b3647 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 2fc6593b1131ee58ed044f526119298bbfd53104..4c1ae42f7af641d2ef1a26d4ce9b058c87931f4b 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..5e08b7666d2fe57f6e5a9eaece17bb63ec2b4d28
--- /dev/null
+++ b/src/sbus/sssd_dbus_utils.c
@@ -0,0 +1,227 @@
+/*
+    Authors:
+        Pavel Březina <pbrez...@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 <pbrez...@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.1.0

From c1a1c8ddd831adb7f43d7b9a3cbdd3c36b080b3d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Wed, 29 Jun 2016 14:03:38 +0200
Subject: [PATCH 4/7] sssctl: use talloc with sifp

This way we completely move D-Bus memory management to talloc and
we reduce number of code lines needed to send and receive reply.
---
 src/tools/sssctl/sssctl.h         | 14 +++++++++
 src/tools/sssctl/sssctl_domains.c | 62 ++++++++++++++++++---------------------
 src/tools/sssctl/sssctl_sifp.c    | 46 +++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+), 34 deletions(-)

diff --git a/src/tools/sssctl/sssctl.h b/src/tools/sssctl/sssctl.h
index 72930ee5c3a1195e90c6e35768f715cbf6a1c4e1..b918dfc1f624d14acc116fd8e3d0943a00d640be 100644
--- a/src/tools/sssctl/sssctl.h
+++ b/src/tools/sssctl/sssctl.h
@@ -24,6 +24,7 @@
 #include "lib/sifp/sss_sifp.h"
 #include "lib/sifp/sss_sifp_dbus.h"
 #include "tools/common/sss_tools.h"
+#include "sbus/sssd_dbus.h"
 
 enum sssctl_prompt_result {
     SSSCTL_PROMPT_YES,
@@ -56,6 +57,19 @@ void _sssctl_sifp_error(sss_sifp_ctx *sifp,
 #define sssctl_sifp_error(sifp, error, message) \
     _sssctl_sifp_error(sifp, error, _(message))
 
+sss_sifp_error _sssctl_sifp_send(TALLOC_CTX *mem_ctx,
+                                 sss_sifp_ctx *sifp,
+                                 DBusMessage **_reply,
+                                 const char *path,
+                                 const char *iface,
+                                 const char *method,
+                                 int first_arg_type,
+                                 ...);
+
+#define sssctl_sifp_send(mem_ctx, sifp, reply, path, iface, method, ...) \
+    _sssctl_sifp_send(mem_ctx, sifp, reply, path, iface, method,         \
+                         ##__VA_ARGS__, DBUS_TYPE_INVALID);
+
 errno_t sssctl_domain_list(struct sss_cmdline *cmdline,
                            struct sss_tool_ctx *tool_ctx,
                            void *pvt);
diff --git a/src/tools/sssctl/sssctl_domains.c b/src/tools/sssctl/sssctl_domains.c
index 17ad670f39dfc045ba090210ffcfa77df713c306..40962792b84eabeb2c142f158184b17180a01669 100644
--- a/src/tools/sssctl/sssctl_domains.c
+++ b/src/tools/sssctl/sssctl_domains.c
@@ -74,47 +74,32 @@ errno_t sssctl_domain_list(struct sss_cmdline *cmdline,
 }
 
 static errno_t sssctl_domain_status_online(struct sss_tool_ctx *tool_ctx,
-                                           const char *domain_path,
-                                           bool force_start)
+                                           sss_sifp_ctx *sifp,
+                                           const char *domain_path)
 {
-    sss_sifp_ctx *sifp;
-    sss_sifp_error sifp_error;
-    DBusMessage *reply = NULL;
-    DBusMessage *msg;
+    TALLOC_CTX *tmp_ctx;
+    sss_sifp_error error;
+    DBusMessage *reply;
     bool is_online;
     errno_t ret;
 
-    if (!sssctl_start_sssd(force_start)) {
-        ret = ERR_SSSD_NOT_RUNNING;
-        goto done;
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
+        return ENOMEM;
     }
 
-    sifp_error = sssctl_sifp_init(tool_ctx, &sifp);
-    if (sifp_error != SSS_SIFP_OK) {
-        sssctl_sifp_error(sifp, sifp_error, "Unable to connect to the InfoPipe");
-        ret = EFAULT;
-        goto done;
-    }
-
-    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");
+    error = sssctl_sifp_send(tmp_ctx, sifp, &reply, domain_path,
+                             IFACE_IFP_DOMAINS_DOMAIN,
+                             IFACE_IFP_DOMAINS_DOMAIN_ISONLINE);
+    if (error != SSS_SIFP_OK) {
+        sssctl_sifp_error(sifp, error, "Unable to get online status");
         ret = EIO;
         goto done;
     }
 
     ret = sbus_parse_reply(reply, DBUS_TYPE_BOOLEAN, &is_online);
     if (ret != EOK) {
-        fprintf(stderr, _("Unable to get information from SSSD\n"));
         goto done;
     }
 
@@ -123,10 +108,7 @@ static errno_t sssctl_domain_status_online(struct sss_tool_ctx *tool_ctx,
     ret = EOK;
 
 done:
-    if (reply != NULL) {
-        dbus_message_unref(reply);
-    }
-
+    talloc_free(tmp_ctx);
     return ret;
 }
 
@@ -144,6 +126,8 @@ errno_t sssctl_domain_status(struct sss_cmdline *cmdline,
                              void *pvt)
 {
     struct sssctl_domain_status_opts opts = {0};
+    sss_sifp_ctx *sifp;
+    sss_sifp_error error;
     const char *path;
     bool opt_set;
     errno_t ret;
@@ -181,7 +165,17 @@ errno_t sssctl_domain_status(struct sss_cmdline *cmdline,
         return ENOMEM;
     }
 
-    ret = sssctl_domain_status_online(tool_ctx, path, opts.force_start);
+    if (!sssctl_start_sssd(opts.force_start)) {
+        return ERR_SSSD_NOT_RUNNING;
+    }
+
+    error = sssctl_sifp_init(tool_ctx, &sifp);
+    if (error != SSS_SIFP_OK) {
+        sssctl_sifp_error(sifp, error, "Unable to connect to the InfoPipe");
+        return EFAULT;
+    }
+
+    ret = sssctl_domain_status_online(tool_ctx, sifp, path);
     if (ret != EOK) {
         fprintf(stderr, _("Unable to get online status\n"));
         return ret;
diff --git a/src/tools/sssctl/sssctl_sifp.c b/src/tools/sssctl/sssctl_sifp.c
index e541c4b27ba38e50b209b0957c8b38f03afc891a..782a72d7ce8bbf1080c6d6ac988ffac2f432955f 100644
--- a/src/tools/sssctl/sssctl_sifp.c
+++ b/src/tools/sssctl/sssctl_sifp.c
@@ -116,3 +116,49 @@ void _sssctl_sifp_error(sss_sifp_ctx *sifp,
         break;
     }
 }
+
+sss_sifp_error _sssctl_sifp_send(TALLOC_CTX *mem_ctx,
+                                 sss_sifp_ctx *sifp,
+                                 DBusMessage **_reply,
+                                 const char *path,
+                                 const char *iface,
+                                 const char *method,
+                                 int first_arg_type,
+                                 ...)
+{
+    sss_sifp_error error;
+    DBusMessage *msg;
+    dbus_bool_t bret;
+    errno_t ret;
+    va_list va;
+
+    msg = sss_sifp_create_message(path, iface, method);
+    if (msg == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create D-Bus message\n");
+        return SSS_SIFP_OUT_OF_MEMORY;
+    }
+
+    va_start(va, first_arg_type);
+    bret = dbus_message_append_args_valist(msg, first_arg_type, va);
+    va_end(va);
+    if (!bret) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Failed to build message\n");
+        error = SSS_SIFP_OUT_OF_MEMORY;
+        goto done;
+    }
+
+    error = sss_sifp_send_message(sifp, msg, _reply);
+    if (error != SSS_SIFP_OK) {
+        goto done;
+    }
+
+    ret = sbus_talloc_bound_message(mem_ctx, *_reply);
+    if (ret != EOK) {
+        error = SSS_SIFP_OUT_OF_MEMORY;
+        goto done;
+    }
+
+done:
+    dbus_message_unref(msg);
+    return error;
+}
-- 
2.1.0

From 171d75faa6d0683d0e19237d7ba538f38e0708ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Wed, 29 Jun 2016 14:58:37 +0200
Subject: [PATCH 5/7] failover: mark subdomain service with sd_ prefix

---
 src/providers/ad/ad_subdomains.c          | 11 +++++++++--
 src/providers/ipa/ipa_subdomains_server.c | 11 +++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index e9da04e384e598927f9c8c203a751bcccd29e895..a0d5c2e544fc62fda64771dce59b3b7ab8ecd8b6 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -66,6 +66,7 @@ ad_subdom_ad_ctx_new(struct be_ctx *be_ctx,
     struct ad_options *ad_options;
     struct ad_id_ctx *ad_id_ctx;
     const char *gc_service_name;
+    const char *service_name;
     struct ad_srv_plugin_ctx *srv_ctx;
     char *ad_domain;
     char *ad_site_override;
@@ -94,14 +95,20 @@ ad_subdom_ad_ctx_new(struct be_ctx *be_ctx,
 
     ad_site_override = dp_opt_get_string(ad_options->basic, AD_SITE);
 
-    gc_service_name = talloc_asprintf(ad_options, "%s%s", "gc_", subdom->name);
+    gc_service_name = talloc_asprintf(ad_options, "sd_gc_%s", subdom->name);
     if (gc_service_name == NULL) {
         talloc_free(ad_options);
         return ENOMEM;
     }
 
+    service_name = talloc_asprintf(ad_options, "sd_%s", subdom->name);
+    if (service_name == NULL) {
+        talloc_free(ad_options);
+        return ENOMEM;
+    }
+
     ret = ad_failover_init(ad_options, be_ctx, NULL, NULL, realm,
-                           subdom->name, gc_service_name,
+                           service_name, gc_service_name,
                            subdom->name, &ad_options->service);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "Cannot initialize AD failover\n");
diff --git a/src/providers/ipa/ipa_subdomains_server.c b/src/providers/ipa/ipa_subdomains_server.c
index a2fed36fbec59100ecca0c1a7d7ca0b3c61f51a1..50f0619ae6946d93b2f703d4929e243adb989cde 100644
--- a/src/providers/ipa/ipa_subdomains_server.c
+++ b/src/providers/ipa/ipa_subdomains_server.c
@@ -203,6 +203,7 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx,
     struct ad_options *ad_options;
     struct ad_id_ctx *ad_id_ctx;
     const char *gc_service_name;
+    const char *service_name;
     struct ad_srv_plugin_ctx *srv_ctx;
     const char *ad_domain;
     const char *ad_site_override;
@@ -250,17 +251,23 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx,
         DEBUG(SSSDBG_TRACE_ALL, "No extra attrs set.\n");
     }
 
-    gc_service_name = talloc_asprintf(ad_options, "%s%s", "gc_", subdom->forest);
+    gc_service_name = talloc_asprintf(ad_options, "sd_gc_%s", subdom->forest);
     if (gc_service_name == NULL) {
         talloc_free(ad_options);
         return ENOMEM;
     }
 
+    service_name = talloc_asprintf(ad_options, "sd_%s", subdom->name);
+    if (service_name == NULL) {
+        talloc_free(ad_options);
+        return ENOMEM;
+    }
+
     /* Set KRB5 realm to same as the one of IPA when IPA
      * is able to attach PAC. For testing, use hardcoded. */
     ret = ad_failover_init(ad_options, be_ctx, NULL, NULL,
                            id_ctx->server_mode->realm,
-                           subdom->name, gc_service_name,
+                           service_name, gc_service_name,
                            subdom->name, &ad_options->service);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "Cannot initialize AD failover\n");
-- 
2.1.0

From d517359421ade7a4c19d6fc3cecd15054868c16d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Mon, 27 Jun 2016 13:56:13 +0200
Subject: [PATCH 6/7] sssctl: print active server and server list

---
 src/providers/data_provider/dp_iface.c           |   4 +-
 src/providers/data_provider/dp_iface.h           |   8 +
 src/providers/data_provider/dp_iface.xml         |   8 +
 src/providers/data_provider/dp_iface_failover.c  | 297 ++++++++++++++++++++++-
 src/providers/data_provider/dp_iface_generated.c |  52 ++++
 src/providers/data_provider/dp_iface_generated.h |  10 +
 src/providers/fail_over.c                        |  42 ++++
 src/providers/fail_over.h                        |   4 +
 src/responder/ifp/ifp_domains.c                  |  46 ++++
 src/responder/ifp/ifp_domains.h                  |   8 +
 src/responder/ifp/ifp_iface.c                    |   4 +-
 src/responder/ifp/ifp_iface.xml                  |  10 +
 src/responder/ifp/ifp_iface_generated.c          |  52 ++++
 src/responder/ifp/ifp_iface_generated.h          |  10 +
 src/tools/sssctl/sssctl_domains.c                | 182 +++++++++++++-
 15 files changed, 719 insertions(+), 18 deletions(-)

diff --git a/src/providers/data_provider/dp_iface.c b/src/providers/data_provider/dp_iface.c
index 8ed7274f0dd7b59598e2cf21e0dd59d16666df0b..1821b44b0a55a6f0c0406efcca4c88a546e2de09 100644
--- a/src/providers/data_provider/dp_iface.c
+++ b/src/providers/data_provider/dp_iface.c
@@ -43,7 +43,9 @@ struct iface_dp_backend iface_dp_backend = {
 
 struct iface_dp_failover iface_dp_failover = {
     {&iface_dp_failover_meta, 0},
-    .ListServices = dp_failover_list_services
+    .ListServices = dp_failover_list_services,
+    .ActiveServer = dp_failover_active_server,
+    .ListServers = dp_failover_list_servers
 };
 
 static struct sbus_iface_map dp_map[] = {
diff --git a/src/providers/data_provider/dp_iface.h b/src/providers/data_provider/dp_iface.h
index 76e623d21c413fd68f8f3c9a91ea32fd707dc54d..5c6f0eb2f5dd68b63bda389e6fdd2446ca9efb21 100644
--- a/src/providers/data_provider/dp_iface.h
+++ b/src/providers/data_provider/dp_iface.h
@@ -69,4 +69,12 @@ errno_t dp_failover_list_services(struct sbus_request *sbus_req,
                                   void *dp_cli,
                                   const char *domname);
 
+errno_t dp_failover_active_server(struct sbus_request *sbus_req,
+                                  void *dp_cli,
+                                  const char *service_name);
+
+errno_t dp_failover_list_servers(struct sbus_request *sbus_req,
+                                 void *dp_cli,
+                                 const char *service_name);
+
 #endif /* DP_IFACE_H_ */
diff --git a/src/providers/data_provider/dp_iface.xml b/src/providers/data_provider/dp_iface.xml
index eab7fc0f1500bf8890030352421da62c134115b9..992848a048ef9fe813d6ae05bbcabd0913ecb277 100644
--- a/src/providers/data_provider/dp_iface.xml
+++ b/src/providers/data_provider/dp_iface.xml
@@ -22,6 +22,14 @@
             <arg name="domain_name" type="s" direction="in" />
             <arg name="services" type="as" direction="out" />
         </method>
+        <method name="ActiveServer">
+            <arg name="service_name" type="s" direction="in" />
+            <arg name="server" type="s" direction="out" />
+        </method>
+        <method name="ListServers">
+            <arg name="service_name" type="s" direction="in" />
+            <arg name="servers" type="as" direction="out" />
+        </method>
     </interface>
 
     <interface name="org.freedesktop.sssd.dataprovider">
diff --git a/src/providers/data_provider/dp_iface_failover.c b/src/providers/data_provider/dp_iface_failover.c
index 038791088eeab7e9c5923996db77d2a107ff067d..388151167a331efa0c59cdabf39fc001d5dc5d83 100644
--- a/src/providers/data_provider/dp_iface_failover.c
+++ b/src/providers/data_provider/dp_iface_failover.c
@@ -28,20 +28,208 @@
 #include "providers/backend.h"
 #include "util/util.h"
 
+static errno_t
+dp_failover_list_services_ldap(struct be_ctx *be_ctx,
+                               const char **services,
+                               int *_count)
+{
+    struct be_svc_data *svc;
+    int count;
+
+    count = 0;
+    DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
+        services[count] = talloc_strdup(services, svc->name);
+        if (services[count] == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup() failed\n");
+            return ENOMEM;
+        }
+        count++;
+    }
+
+    *_count = count;
+
+    return EOK;
+}
+
+static errno_t
+dp_failover_list_services_ad(struct be_ctx *be_ctx,
+                             struct sss_domain_info *domain,
+                             const char **services,
+                             int *_count)
+{
+    char *fo_svc_name = NULL;
+    struct be_svc_data *svc;
+    errno_t ret;
+    int count;
+
+    fo_svc_name = talloc_asprintf(services, "sd_%s", domain->name);
+    if (fo_svc_name == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    count = 0;
+    DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
+        /* Drop each sd_gc_* since this service is not used with AD at all,
+         * we only connect to AD_GC for global catalog. */
+        if (strncasecmp(svc->name, "sd_gc_", strlen("sd_gc_")) == 0) {
+            continue;
+        }
+
+        /* Drop all subdomain services for different domain. */
+        if (strncasecmp(svc->name, "sd_", strlen("sd_")) == 0) {
+            if (!IS_SUBDOMAIN(domain)) {
+                continue;
+            }
+
+            if (strcasecmp(svc->name, fo_svc_name) != 0) {
+                continue;
+            }
+        }
+
+        if (IS_SUBDOMAIN(domain)) {
+            /* Drop AD since we connect to subdomain.com for LDAP. */
+            if (strcasecmp(svc->name, "AD") == 0) {
+                continue;
+            }
+        }
+
+        services[count] = talloc_strdup(services, svc->name);
+        if (services[count] == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup() failed\n");
+            ret = ENOMEM;
+            goto done;
+        }
+        count++;
+    }
+
+    *_count = count;
+
+    ret = EOK;
+
+done:
+    talloc_free(fo_svc_name);
+    return ret;
+}
+
+static errno_t
+dp_failover_list_services_ipa(struct be_ctx *be_ctx,
+                              struct sss_domain_info *domain,
+                              const char **services,
+                              int *_count)
+{
+    struct be_svc_data *svc;
+    char *fo_svc_name = NULL;
+    char *fo_gc_name = NULL;
+    errno_t ret;
+    int count;
+
+    fo_svc_name = talloc_asprintf(services, "sd_%s", domain->name);
+    if (fo_svc_name == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    fo_gc_name = talloc_asprintf(services, "sd_gc_%s", domain->name);
+    if (fo_gc_name == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    count = 0;
+    DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
+        /* Drop all subdomain services for different domain. */
+        if (strncasecmp(svc->name, "sd_", strlen("sd_")) == 0) {
+            if (!IS_SUBDOMAIN(domain)) {
+                continue;
+            }
+
+            if (strcasecmp(svc->name, fo_svc_name) != 0
+                    && strcasecmp(svc->name, fo_gc_name) != 0) {
+                continue;
+            }
+        }
+
+        services[count] = talloc_strdup(services, svc->name);
+        if (services[count] == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup() failed\n");
+            return ENOMEM;
+        }
+        count++;
+    }
+
+    *_count = count;
+
+    ret = EOK;
+
+done:
+    talloc_free(fo_svc_name);
+    talloc_free(fo_gc_name);
+
+    return ret;
+}
+
+enum dp_fo_svc_type {
+    DP_FO_SVC_LDAP = 0,
+    DP_FO_SVC_AD = 1,
+    DP_FO_SVC_IPA = 1 << 1,
+    DP_FO_SVC_MIXED = DP_FO_SVC_AD | DP_FO_SVC_IPA
+};
+
 errno_t dp_failover_list_services(struct sbus_request *sbus_req,
                                   void *dp_cli,
                                   const char *domname)
 {
+    enum dp_fo_svc_type svc_type = DP_FO_SVC_LDAP;
+    struct sss_domain_info *domain;
     struct be_ctx *be_ctx;
     struct be_svc_data *svc;
     const char **services;
     int num_services;
+    errno_t ret;
 
     be_ctx = dp_client_be(dp_cli);
 
+    if (SBUS_IS_STRING_EMPTY(domname)) {
+        domain = be_ctx->domain;
+    } else {
+        domain = find_domain_by_name(be_ctx->domain, domname, false);
+        if (domain == NULL) {
+            sbus_request_reply_error(sbus_req, SBUS_ERROR_UNKNOWN_DOMAIN,
+                                     "Unknown domain %s", domname);
+            return EOK;
+        }
+    }
+
+    /**
+     * Returning list of failover services is currently rather difficult
+     * since there is only one failover context for the whole backend.
+     *
+     * The list of services for the given domain depends on whether it is
+     * a master domain or a subdomain and whether we are using IPA, AD or
+     * LDAP backend.
+     *
+     * For LDAP we just return everything we have.
+     * For AD master domain we return AD, AD_GC.
+     * For AD subdomain we return subdomain.com, AD_GC.
+     * For IPA in client mode we return IPA.
+     * For IPA in server mode we return IPA for master domain and
+     * subdomain.com, gc_subdomain.com for subdomain.
+     *
+     * We also return everything else for all cases if any other service
+     * such as kerberos is configured separately.
+     */
+
+    /* Allocate enough space. */
     num_services = 0;
     DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
         num_services++;
+
+        if (strcasecmp(svc->name, "AD") == 0) {
+            svc_type |= DP_FO_SVC_AD;
+        } else if (strcasecmp(svc->name, "IPA") == 0) {
+            svc_type |= DP_FO_SVC_IPA;
+        }
     }
 
     services = talloc_zero_array(sbus_req, const char *, num_services);
@@ -50,17 +238,108 @@ errno_t dp_failover_list_services(struct sbus_request *sbus_req,
         return ENOMEM;
     }
 
-    num_services = 0;
-    DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
-        services[num_services] = talloc_strdup(services, svc->name);
-        if (services[num_services] == NULL) {
-            DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup() failed\n");
-            talloc_free(services);
-            return ENOMEM;
-        }
-        num_services++;
+    /* Fill the list. */
+    switch (svc_type) {
+    case DP_FO_SVC_LDAP:
+    case DP_FO_SVC_MIXED:
+        ret = dp_failover_list_services_ldap(be_ctx, services, &num_services);
+        break;
+    case DP_FO_SVC_AD:
+        ret = dp_failover_list_services_ad(be_ctx, domain,
+                                           services, &num_services);
+        break;
+    case DP_FO_SVC_IPA:
+        ret = dp_failover_list_services_ipa(be_ctx, domain,
+                                            services, &num_services);
+        break;
+    default:
+        ret = ERR_INTERNAL;
+        break;
+    }
+
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create service list [%d]: %s\n",
+              ret, sss_strerror(ret));
+        talloc_free(services);
+        return ret;
     }
 
     iface_dp_failover_ListServices_finish(sbus_req, services, num_services);
     return EOK;
 }
+
+errno_t dp_failover_active_server(struct sbus_request *sbus_req,
+                                  void *dp_cli,
+                                  const char *service_name)
+{
+    struct be_ctx *be_ctx;
+    struct be_svc_data *svc;
+    const char *server;
+    bool found = false;
+
+    be_ctx = dp_client_be(dp_cli);
+
+    DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
+        if (strcmp(svc->name, service_name) == 0) {
+            found = true;
+            break;
+        }
+    }
+
+    if (!found) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get server name\n");
+        sbus_request_reply_error(sbus_req, SBUS_ERROR_NOT_FOUND,
+                                 "Unknown service name");
+        return EOK;
+    }
+
+    if (svc->last_good_srv == NULL) {
+        server = "";
+    } else {
+        server = fo_get_server_name(svc->last_good_srv);
+        if (server == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get server name\n");
+            sbus_request_reply_error(sbus_req, SBUS_ERROR_INTERNAL,
+                                     "Unable to get server name");
+            return EOK;
+        }
+    }
+
+    iface_dp_failover_ActiveServer_finish(sbus_req, server);
+    return EOK;
+}
+
+errno_t dp_failover_list_servers(struct sbus_request *sbus_req,
+                                 void *dp_cli,
+                                 const char *service_name)
+{
+    struct be_ctx *be_ctx;
+    struct be_svc_data *svc;
+    const char **servers;
+    bool found = false;
+    size_t count;;
+
+    be_ctx = dp_client_be(dp_cli);
+
+    DLIST_FOR_EACH(svc, be_ctx->be_fo->svcs) {
+        if (strcmp(svc->name, service_name) == 0) {
+            found = true;
+            break;
+        }
+    }
+
+    if (!found) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get server list\n");
+        sbus_request_reply_error(sbus_req, SBUS_ERROR_NOT_FOUND,
+                                 "Unknown service name");
+        return EOK;
+    }
+
+    servers = fo_svc_server_list(sbus_req, svc->fo_service, &count);
+    if (servers == NULL) {
+        return ENOMEM;
+    }
+
+    iface_dp_failover_ListServers_finish(sbus_req, servers, (int)count);
+    return EOK;
+}
diff --git a/src/providers/data_provider/dp_iface_generated.c b/src/providers/data_provider/dp_iface_generated.c
index 7b36fd8aaeebb976a511c5592b1dd0ae28e9bb8a..fd2acb4f4bd8cf1dcbe8842cccc6dc2077fc83a2 100644
--- a/src/providers/data_provider/dp_iface_generated.c
+++ b/src/providers/data_provider/dp_iface_generated.c
@@ -111,6 +111,44 @@ int iface_dp_failover_ListServices_finish(struct sbus_request *req, const char *
                                          DBUS_TYPE_INVALID);
 }
 
+/* arguments for org.freedesktop.sssd.DataProvider.Failover.ActiveServer */
+const struct sbus_arg_meta iface_dp_failover_ActiveServer__in[] = {
+    { "service_name", "s" },
+    { NULL, }
+};
+
+/* arguments for org.freedesktop.sssd.DataProvider.Failover.ActiveServer */
+const struct sbus_arg_meta iface_dp_failover_ActiveServer__out[] = {
+    { "server", "s" },
+    { NULL, }
+};
+
+int iface_dp_failover_ActiveServer_finish(struct sbus_request *req, const char *arg_server)
+{
+   return sbus_request_return_and_finish(req,
+                                         DBUS_TYPE_STRING, &arg_server,
+                                         DBUS_TYPE_INVALID);
+}
+
+/* arguments for org.freedesktop.sssd.DataProvider.Failover.ListServers */
+const struct sbus_arg_meta iface_dp_failover_ListServers__in[] = {
+    { "service_name", "s" },
+    { NULL, }
+};
+
+/* arguments for org.freedesktop.sssd.DataProvider.Failover.ListServers */
+const struct sbus_arg_meta iface_dp_failover_ListServers__out[] = {
+    { "servers", "as" },
+    { NULL, }
+};
+
+int iface_dp_failover_ListServers_finish(struct sbus_request *req, const char *arg_servers[], int len_servers)
+{
+   return sbus_request_return_and_finish(req,
+                                         DBUS_TYPE_ARRAY, DBUS_TYPE_STRING, &arg_servers, len_servers,
+                                         DBUS_TYPE_INVALID);
+}
+
 /* methods for org.freedesktop.sssd.DataProvider.Failover */
 const struct sbus_method_meta iface_dp_failover__methods[] = {
     {
@@ -120,6 +158,20 @@ const struct sbus_method_meta iface_dp_failover__methods[] = {
         offsetof(struct iface_dp_failover, ListServices),
         invoke_s_method,
     },
+    {
+        "ActiveServer", /* name */
+        iface_dp_failover_ActiveServer__in,
+        iface_dp_failover_ActiveServer__out,
+        offsetof(struct iface_dp_failover, ActiveServer),
+        invoke_s_method,
+    },
+    {
+        "ListServers", /* name */
+        iface_dp_failover_ListServers__in,
+        iface_dp_failover_ListServers__out,
+        offsetof(struct iface_dp_failover, ListServers),
+        invoke_s_method,
+    },
     { NULL, }
 };
 
diff --git a/src/providers/data_provider/dp_iface_generated.h b/src/providers/data_provider/dp_iface_generated.h
index 977ab3bae803ca002162b02d0c3d9677779983f4..7c2216aa27022769c707d80b59e9b436e72d1739 100644
--- a/src/providers/data_provider/dp_iface_generated.h
+++ b/src/providers/data_provider/dp_iface_generated.h
@@ -22,6 +22,8 @@
 /* constants for org.freedesktop.sssd.DataProvider.Failover */
 #define IFACE_DP_FAILOVER "org.freedesktop.sssd.DataProvider.Failover"
 #define IFACE_DP_FAILOVER_LISTSERVICES "ListServices"
+#define IFACE_DP_FAILOVER_ACTIVESERVER "ActiveServer"
+#define IFACE_DP_FAILOVER_LISTSERVERS "ListServers"
 
 /* constants for org.freedesktop.sssd.dataprovider */
 #define IFACE_DP "org.freedesktop.sssd.dataprovider"
@@ -72,11 +74,19 @@ int iface_dp_backend_IsOnline_finish(struct sbus_request *req, bool arg_status);
 struct iface_dp_failover {
     struct sbus_vtable vtable; /* derive from sbus_vtable */
     int (*ListServices)(struct sbus_request *req, void *data, const char *arg_domain_name);
+    int (*ActiveServer)(struct sbus_request *req, void *data, const char *arg_service_name);
+    int (*ListServers)(struct sbus_request *req, void *data, const char *arg_service_name);
 };
 
 /* finish function for ListServices */
 int iface_dp_failover_ListServices_finish(struct sbus_request *req, const char *arg_services[], int len_services);
 
+/* finish function for ActiveServer */
+int iface_dp_failover_ActiveServer_finish(struct sbus_request *req, const char *arg_server);
+
+/* finish function for ListServers */
+int iface_dp_failover_ListServers_finish(struct sbus_request *req, const char *arg_servers[], int len_servers);
+
 /* vtable for org.freedesktop.sssd.dataprovider */
 struct iface_dp {
     struct sbus_vtable vtable; /* derive from sbus_vtable */
diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index 1d88d2aa54bfdebd4b648e2b13fa8d03e2be3973..8ab39f27f77e19e601855632196006a8dbbdf136 100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -1660,6 +1660,48 @@ bool fo_svc_has_server(struct fo_service *service, struct fo_server *server)
     return false;
 }
 
+const char **fo_svc_server_list(TALLOC_CTX *mem_ctx,
+                                struct fo_service *service,
+                                size_t *_count)
+{
+    const char **list;
+    const char *server;
+    struct fo_server *srv;
+    size_t count;
+
+    count = 0;
+    DLIST_FOR_EACH(srv, service->server_list) {
+        count++;
+    }
+
+    list = talloc_zero_array(mem_ctx, const char *, count + 1);
+    if (list == NULL) {
+        return NULL;
+    }
+
+    count = 0;
+    DLIST_FOR_EACH(srv, service->server_list) {
+        server = fo_get_server_name(srv);
+        if (server == NULL) {
+            /* _srv_ */
+            continue;
+        }
+
+        list[count] = talloc_strdup(list, server);
+        if (list[count] == NULL) {
+            talloc_free(list);
+            return NULL;
+        }
+        count++;
+    }
+
+    if (_count != NULL) {
+        *_count = count;
+    }
+
+    return list;
+}
+
 bool fo_set_srv_lookup_plugin(struct fo_ctx *ctx,
                               fo_srv_lookup_plugin_send_t send_fn,
                               fo_srv_lookup_plugin_recv_t recv_fn,
diff --git a/src/providers/fail_over.h b/src/providers/fail_over.h
index f24b5715f13931965400c20562a1578aaf756908..d70212fb7ea569b9c47bba36704aa8ae18754cbb 100644
--- a/src/providers/fail_over.h
+++ b/src/providers/fail_over.h
@@ -212,6 +212,10 @@ struct fo_server *fo_get_active_server(struct fo_service *service);
 
 bool fo_svc_has_server(struct fo_service *service, struct fo_server *server);
 
+const char **fo_svc_server_list(TALLOC_CTX *mem_ctx,
+                                struct fo_service *service,
+                                size_t *_count);
+
 /*
  * pvt will be talloc_stealed to ctx
  */
diff --git a/src/responder/ifp/ifp_domains.c b/src/responder/ifp/ifp_domains.c
index 402f2f086bc6bb60f7736e685e124694cebc14ca..22a8f5fdca0ee38223e4330efe78ab5d88e21ae4 100644
--- a/src/responder/ifp/ifp_domains.c
+++ b/src/responder/ifp/ifp_domains.c
@@ -581,3 +581,49 @@ int ifp_domains_domain_list_services(struct sbus_request *sbus_req,
 
     return EOK;
 }
+
+int ifp_domains_domain_active_server(struct sbus_request *sbus_req,
+                                     void *data,
+                                     const char *service)
+{
+    struct ifp_ctx *ifp_ctx;
+    struct sss_domain_info *dom;
+
+    ifp_ctx = talloc_get_type(data, struct ifp_ctx);
+
+    dom = get_domain_info_from_req(sbus_req, data);
+    if (dom == NULL) {
+        sbus_request_reply_error(sbus_req, SBUS_ERROR_UNKNOWN_DOMAIN,
+                                 "Unknown domain");
+        return EOK;
+    }
+
+    rdp_message_send_and_reply(sbus_req, ifp_ctx->rctx, dom, DP_PATH,
+                           IFACE_DP_FAILOVER, IFACE_DP_FAILOVER_ACTIVESERVER,
+                           DBUS_TYPE_STRING, &service);
+
+    return EOK;
+}
+
+int ifp_domains_domain_list_servers(struct sbus_request *sbus_req,
+                                    void *data,
+                                    const char *service)
+{
+    struct ifp_ctx *ifp_ctx;
+    struct sss_domain_info *dom;
+
+    ifp_ctx = talloc_get_type(data, struct ifp_ctx);
+
+    dom = get_domain_info_from_req(sbus_req, data);
+    if (dom == NULL) {
+        sbus_request_reply_error(sbus_req, SBUS_ERROR_UNKNOWN_DOMAIN,
+                                 "Unknown domain");
+        return EOK;
+    }
+
+    rdp_message_send_and_reply(sbus_req, ifp_ctx->rctx, dom, DP_PATH,
+                           IFACE_DP_FAILOVER, IFACE_DP_FAILOVER_LISTSERVERS,
+                           DBUS_TYPE_STRING, &service);
+
+    return EOK;
+}
diff --git a/src/responder/ifp/ifp_domains.h b/src/responder/ifp/ifp_domains.h
index 91645e60701f8f75e89a42e93e2c066def67b018..621ba6158e285911cb8298cef212219dfd3afec8 100644
--- a/src/responder/ifp/ifp_domains.h
+++ b/src/responder/ifp/ifp_domains.h
@@ -100,4 +100,12 @@ int ifp_domains_domain_is_online(struct sbus_request *sbus_req,
 int ifp_domains_domain_list_services(struct sbus_request *sbus_req,
                                      void *data);
 
+int ifp_domains_domain_active_server(struct sbus_request *sbus_req,
+                                     void *data,
+                                     const char *service);
+
+int ifp_domains_domain_list_servers(struct sbus_request *sbus_req,
+                                    void *data,
+                                    const char *service);
+
 #endif /* IFP_DOMAINS_H_ */
diff --git a/src/responder/ifp/ifp_iface.c b/src/responder/ifp/ifp_iface.c
index 90bb52b2ccf5207034abbe12bddbfa1eeaf875f7..e6ddc687ba9db878ee39fee5868d1f924d58482d 100644
--- a/src/responder/ifp/ifp_iface.c
+++ b/src/responder/ifp/ifp_iface.c
@@ -81,7 +81,9 @@ struct iface_ifp_domains iface_ifp_domains = {
 struct iface_ifp_domains_domain iface_ifp_domains_domain = {
     { &iface_ifp_domains_domain_meta, 0 },
     .IsOnline = ifp_domains_domain_is_online,
-    .ListServices = ifp_domains_domain_list_services
+    .ListServices = ifp_domains_domain_list_services,
+    .ActiveServer = ifp_domains_domain_active_server,
+    .ListServers = ifp_domains_domain_list_servers
 };
 
 struct iface_ifp_users iface_ifp_users = {
diff --git a/src/responder/ifp/ifp_iface.xml b/src/responder/ifp/ifp_iface.xml
index 7f6f47299deeba4b1baa23d1e63ee7bb17304a59..25b104ad70c0fd84b6c0fe9dbb0dc6e6439c1376 100644
--- a/src/responder/ifp/ifp_iface.xml
+++ b/src/responder/ifp/ifp_iface.xml
@@ -112,6 +112,16 @@
         <method name="ListServices">
             <arg name="services" type="as" direction="out" />
         </method>
+
+        <method name="ActiveServer">
+            <arg name="service" type="s" direction="in" />
+            <arg name="server" type="s" direction="out" />
+        </method>
+
+        <method name="ListServers">
+            <arg name="service_name" type="s" direction="in" />
+            <arg name="servers" type="as" direction="out" />
+        </method>
     </interface>
 
     <interface name="org.freedesktop.sssd.infopipe.Cache">
diff --git a/src/responder/ifp/ifp_iface_generated.c b/src/responder/ifp/ifp_iface_generated.c
index 4d3bb5727b03ae64adad14fcdbb3eb5366edb406..6156ca2947434f301d206232f83cfc0647007707 100644
--- a/src/responder/ifp/ifp_iface_generated.c
+++ b/src/responder/ifp/ifp_iface_generated.c
@@ -558,6 +558,44 @@ int iface_ifp_domains_domain_ListServices_finish(struct sbus_request *req, const
                                          DBUS_TYPE_INVALID);
 }
 
+/* arguments for org.freedesktop.sssd.infopipe.Domains.Domain.ActiveServer */
+const struct sbus_arg_meta iface_ifp_domains_domain_ActiveServer__in[] = {
+    { "service", "s" },
+    { NULL, }
+};
+
+/* arguments for org.freedesktop.sssd.infopipe.Domains.Domain.ActiveServer */
+const struct sbus_arg_meta iface_ifp_domains_domain_ActiveServer__out[] = {
+    { "server", "s" },
+    { NULL, }
+};
+
+int iface_ifp_domains_domain_ActiveServer_finish(struct sbus_request *req, const char *arg_server)
+{
+   return sbus_request_return_and_finish(req,
+                                         DBUS_TYPE_STRING, &arg_server,
+                                         DBUS_TYPE_INVALID);
+}
+
+/* arguments for org.freedesktop.sssd.infopipe.Domains.Domain.ListServers */
+const struct sbus_arg_meta iface_ifp_domains_domain_ListServers__in[] = {
+    { "service_name", "s" },
+    { NULL, }
+};
+
+/* arguments for org.freedesktop.sssd.infopipe.Domains.Domain.ListServers */
+const struct sbus_arg_meta iface_ifp_domains_domain_ListServers__out[] = {
+    { "servers", "as" },
+    { NULL, }
+};
+
+int iface_ifp_domains_domain_ListServers_finish(struct sbus_request *req, const char *arg_servers[], int len_servers)
+{
+   return sbus_request_return_and_finish(req,
+                                         DBUS_TYPE_ARRAY, DBUS_TYPE_STRING, &arg_servers, len_servers,
+                                         DBUS_TYPE_INVALID);
+}
+
 /* methods for org.freedesktop.sssd.infopipe.Domains.Domain */
 const struct sbus_method_meta iface_ifp_domains_domain__methods[] = {
     {
@@ -574,6 +612,20 @@ const struct sbus_method_meta iface_ifp_domains_domain__methods[] = {
         offsetof(struct iface_ifp_domains_domain, ListServices),
         NULL, /* no invoker */
     },
+    {
+        "ActiveServer", /* name */
+        iface_ifp_domains_domain_ActiveServer__in,
+        iface_ifp_domains_domain_ActiveServer__out,
+        offsetof(struct iface_ifp_domains_domain, ActiveServer),
+        invoke_s_method,
+    },
+    {
+        "ListServers", /* name */
+        iface_ifp_domains_domain_ListServers__in,
+        iface_ifp_domains_domain_ListServers__out,
+        offsetof(struct iface_ifp_domains_domain, ListServers),
+        invoke_s_method,
+    },
     { NULL, }
 };
 
diff --git a/src/responder/ifp/ifp_iface_generated.h b/src/responder/ifp/ifp_iface_generated.h
index 2eff57410e5292a05818050b96eb85aa3a4f2e16..141348249d2da5447fa04495564a8c6a55d67a1b 100644
--- a/src/responder/ifp/ifp_iface_generated.h
+++ b/src/responder/ifp/ifp_iface_generated.h
@@ -58,6 +58,8 @@
 #define IFACE_IFP_DOMAINS_DOMAIN "org.freedesktop.sssd.infopipe.Domains.Domain"
 #define IFACE_IFP_DOMAINS_DOMAIN_ISONLINE "IsOnline"
 #define IFACE_IFP_DOMAINS_DOMAIN_LISTSERVICES "ListServices"
+#define IFACE_IFP_DOMAINS_DOMAIN_ACTIVESERVER "ActiveServer"
+#define IFACE_IFP_DOMAINS_DOMAIN_LISTSERVERS "ListServers"
 
 /* constants for org.freedesktop.sssd.infopipe.Cache */
 #define IFACE_IFP_CACHE "org.freedesktop.sssd.infopipe.Cache"
@@ -215,6 +217,8 @@ struct iface_ifp_domains_domain {
     struct sbus_vtable vtable; /* derive from sbus_vtable */
     int (*IsOnline)(struct sbus_request *req, void *data);
     int (*ListServices)(struct sbus_request *req, void *data);
+    int (*ActiveServer)(struct sbus_request *req, void *data, const char *arg_service);
+    int (*ListServers)(struct sbus_request *req, void *data, const char *arg_service_name);
 };
 
 /* finish function for IsOnline */
@@ -223,6 +227,12 @@ int iface_ifp_domains_domain_IsOnline_finish(struct sbus_request *req, bool arg_
 /* finish function for ListServices */
 int iface_ifp_domains_domain_ListServices_finish(struct sbus_request *req, const char *arg_services[], int len_services);
 
+/* finish function for ActiveServer */
+int iface_ifp_domains_domain_ActiveServer_finish(struct sbus_request *req, const char *arg_server);
+
+/* finish function for ListServers */
+int iface_ifp_domains_domain_ListServers_finish(struct sbus_request *req, const char *arg_servers[], int len_servers);
+
 /* vtable for org.freedesktop.sssd.infopipe.Cache */
 struct iface_ifp_cache {
     struct sbus_vtable vtable; /* derive from sbus_vtable */
diff --git a/src/tools/sssctl/sssctl_domains.c b/src/tools/sssctl/sssctl_domains.c
index 40962792b84eabeb2c142f158184b17180a01669..a12bb402ec38616e5273edf20496d8393a605d38 100644
--- a/src/tools/sssctl/sssctl_domains.c
+++ b/src/tools/sssctl/sssctl_domains.c
@@ -112,6 +112,155 @@ done:
     return ret;
 }
 
+static const char *proper_service_name(const char *service)
+{
+    if (strcasecmp(service, "AD_GC") == 0) {
+        return "AD Global Catalog";
+    } else if (strcasecmp(service, "AD") == 0) {
+        return "AD Domain Controller";
+    } else if (strncasecmp(service, "sd_gc_", strlen("sd_gc_")) == 0) {
+        return "AD Global Catalog";
+    } else if (strncasecmp(service, "sd_", strlen("sd_")) == 0) {
+        return "AD Domain Controller";
+    }
+
+    return service;
+}
+
+static errno_t sssctl_domain_status_active_server(struct sss_tool_ctx *tool_ctx,
+                                                  sss_sifp_ctx *sifp,
+                                                  const char *domain_path)
+{
+    TALLOC_CTX *tmp_ctx;
+    sss_sifp_error error;
+    DBusMessage *reply;
+    const char *server;
+    const char **services;
+    int num_services;
+    errno_t ret;
+    int i;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
+        return ENOMEM;
+    }
+
+    error = sssctl_sifp_send(tmp_ctx, sifp, &reply, domain_path,
+                             IFACE_IFP_DOMAINS_DOMAIN,
+                             IFACE_IFP_DOMAINS_DOMAIN_LISTSERVICES);
+    if (error != SSS_SIFP_OK) {
+        sssctl_sifp_error(sifp, error, "Unable to list services");
+        ret = EIO;
+        goto done;
+    }
+
+    ret = sbus_parse_reply(reply, DBUS_TYPE_ARRAY, DBUS_TYPE_STRING,
+                          &services, &num_services);
+    if (ret != EOK) {
+        goto done;
+    }
+
+    printf(_("Active servers:\n"));
+    for (i = 0; i < num_services; i++) {
+        error = sssctl_sifp_send(tmp_ctx, sifp, &reply, domain_path,
+                                 IFACE_IFP_DOMAINS_DOMAIN,
+                                 IFACE_IFP_DOMAINS_DOMAIN_ACTIVESERVER,
+                                 DBUS_TYPE_STRING, &services[i]);
+        if (error != SSS_SIFP_OK) {
+            sssctl_sifp_error(sifp, error, "Unable to get active server");
+            ret = EIO;
+            goto done;
+        }
+
+        ret = sbus_parse_reply(reply, DBUS_TYPE_STRING, &server);
+        if (ret != EOK) {
+            goto done;
+        }
+
+        server = SBUS_IS_STRING_EMPTY(server) ? _("not connected") : server;
+        printf("%s: %s\n", proper_service_name(services[i]), server);
+    }
+
+    ret = EOK;
+
+done:
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
+static errno_t sssctl_domain_status_server_list(struct sss_tool_ctx *tool_ctx,
+                                                sss_sifp_ctx *sifp,
+                                                const char *domain_path)
+{
+    TALLOC_CTX *tmp_ctx;
+    sss_sifp_error error;
+    DBusMessage *reply;
+    const char **servers;
+    int num_servers;
+    const char **services;
+    int num_services;
+    errno_t ret;
+    int i, j;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
+        return ENOMEM;
+    }
+
+    error = sssctl_sifp_send(tmp_ctx, sifp, &reply, domain_path,
+                             IFACE_IFP_DOMAINS_DOMAIN,
+                             IFACE_IFP_DOMAINS_DOMAIN_LISTSERVICES);
+    if (error != SSS_SIFP_OK) {
+        sssctl_sifp_error(sifp, error, "Unable to list services");
+        ret = EIO;
+        goto done;
+    }
+
+    ret = sbus_parse_reply(reply, DBUS_TYPE_ARRAY, DBUS_TYPE_STRING,
+                          &services, &num_services);
+    if (ret != EOK) {
+        goto done;
+    }
+
+    for (i = 0; i < num_services; i++) {
+        printf(_("Discovered %s servers:\n"), proper_service_name(services[i]));
+        error = sssctl_sifp_send(tmp_ctx, sifp, &reply, domain_path,
+                                 IFACE_IFP_DOMAINS_DOMAIN,
+                                 IFACE_IFP_DOMAINS_DOMAIN_LISTSERVERS,
+                                 DBUS_TYPE_STRING, &services[i]);
+        if (error != SSS_SIFP_OK) {
+            sssctl_sifp_error(sifp, error, "Unable to get active server");
+            ret = EIO;
+            goto done;
+        }
+
+        ret = sbus_parse_reply(reply, DBUS_TYPE_ARRAY, DBUS_TYPE_STRING,
+                               &servers, &num_servers);
+        if (ret != EOK) {
+            goto done;
+        }
+
+        if (num_servers == 0) {
+            puts(_("None so far.\n"));
+            continue;
+        }
+
+        for (j = 0; j < num_servers; j++) {
+            printf("- %s\n", servers[j]);
+        }
+
+        printf("\n");
+    }
+
+    ret = EOK;
+
+done:
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
 struct sssctl_domain_status_opts {
     const char *domain;
     int online;
@@ -135,11 +284,8 @@ errno_t sssctl_domain_status(struct sss_cmdline *cmdline,
     /* Parse command line. */
     struct poptOption options[] = {
         {"online", 'o', POPT_ARG_NONE , &opts.online, 0, _("Show online status"), NULL },
-        /*
-        {"last-requests", 'l', POPT_ARG_NONE, &opts.last, 0, _("Show last requests that went to data provider"), NULL },
         {"active-server", 'a', POPT_ARG_NONE, &opts.active, 0, _("Show information about active server"), NULL },
         {"servers", 'r', POPT_ARG_NONE, &opts.servers, 0, _("Show list of discovered servers"), NULL },
-        */
         {"start", 's', POPT_ARG_NONE, &opts.force_start, 0, _("Start SSSD if it is not running"), NULL },
         POPT_TABLEEND
     };
@@ -175,10 +321,32 @@ errno_t sssctl_domain_status(struct sss_cmdline *cmdline,
         return EFAULT;
     }
 
-    ret = sssctl_domain_status_online(tool_ctx, sifp, path);
-    if (ret != EOK) {
-        fprintf(stderr, _("Unable to get online status\n"));
-        return ret;
+    if (opts.online) {
+        ret = sssctl_domain_status_online(tool_ctx, sifp, path);
+        if (ret != EOK) {
+            fprintf(stderr, _("Unable to get online status\n"));
+            return ret;
+        }
+
+        printf("\n");
+    }
+
+    if (opts.active) {
+        ret = sssctl_domain_status_active_server(tool_ctx, sifp, path);
+        if (ret != EOK) {
+            fprintf(stderr, _("Unable to get online status\n"));
+            return ret;
+        }
+
+        printf("\n");
+    }
+
+    if (opts.servers) {
+        ret = sssctl_domain_status_server_list(tool_ctx, sifp, path);
+        if (ret != EOK) {
+            fprintf(stderr, _("Unable to get server list\n"));
+            return ret;
+        }
     }
 
     return EOK;
-- 
2.1.0

From 531bd907add54e93032d5357593c65ad40b759b2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Thu, 14 Jul 2016 10:49:37 +0200
Subject: [PATCH 7/7] sifp: fix coverity warning

sssd-1.14.1/src/lib/sifp/sss_sifp_dbus.c:51: check_return: Calling "dbus_message_append_args_valist" without checking return value (as is done elsewhere 4 out of 5 times).
---
 src/lib/sifp/sss_sifp_dbus.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/lib/sifp/sss_sifp_dbus.c b/src/lib/sifp/sss_sifp_dbus.c
index 7c72c52f0d226ccdfaf7b8ffaed7776647a7771c..2906c5ac383c412231127f6ffa8081d47eb2bced 100644
--- a/src/lib/sifp/sss_sifp_dbus.c
+++ b/src/lib/sifp/sss_sifp_dbus.c
@@ -36,6 +36,7 @@ static sss_sifp_error sss_sifp_ifp_call(sss_sifp_ctx *ctx,
 {
    DBusMessage *msg = NULL;
    sss_sifp_error ret;
+   dbus_bool_t bret;
 
    if (object_path == NULL || interface == NULL || method == NULL) {
        return SSS_SIFP_INVALID_ARGUMENT;
@@ -48,7 +49,11 @@ static sss_sifp_error sss_sifp_ifp_call(sss_sifp_ctx *ctx,
    }
 
    if (first_arg_type != DBUS_TYPE_INVALID) {
-       dbus_message_append_args_valist(msg, first_arg_type, ap);
+       bret = dbus_message_append_args_valist(msg, first_arg_type, ap);
+       if (!bret) {
+           ret = SSS_SIFP_IO_ERROR;
+           goto done;
+       }
    }
 
    ret = sss_sifp_send_message(ctx, msg, _reply);
-- 
2.1.0

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to