[SSSD] [PATCH] Four small patches related to GC lookups

Lukas Slebodnik lslebodn at redhat.com
Tue May 28 08:30:53 UTC 2013


On (27/05/13 20:37), Jakub Hrozek wrote:
>Hi,
>
>I fond these small issues while working on the GC lookups. I'm sending
>them separately b/c they can be reviewed without any knowledge about GC
>and do not break any existing functionality.
>
>[PATCH 1/4] IPA: Check for ENOMEM
>Just an allocation check.
>
>[PATCH 2/4] Remove unneeded comment
>Not needed anymore I think
>
>[PATCH 3/4] FO: Fix setting status of duplicate
>The fo_set_port_status() function was performing its own duplicate
>detection which is wrong because it duplicates existing code and doesn't
>take the private user_data structure into account.
>
>[PATCH 4/4] AD dyndns: extract the host name from URI
>Because the AD provider might connect to LDAP on different ports
>depending on whether it is a Global Catalog or pure LDAP, we need to
>extract the hostname using LDAP function calls, not just grab the server
>name part from the URI.

>From 151dbd2db82e46369ce8c696a2ab6424683bafc1 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Wed, 22 May 2013 17:03:33 +0200
>Subject: [PATCH 1/4] IPA: Check for ENOMEM
>
>---
> src/providers/ipa/ipa_subdomains.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
>index 98fc69f14d62d74fb82b0f067307362e819aa82c..9c8a926c177ce751466b21b9043920580f70af3c 100644
>--- a/src/providers/ipa/ipa_subdomains.c
>+++ b/src/providers/ipa/ipa_subdomains.c
>@@ -216,6 +216,10 @@ static errno_t ipa_subdom_store(struct sss_domain_info *domain,
>     int ret;
> 
>     tmp_ctx = talloc_new(domain);
>+    if (tmp_ctx == NULL) {
>+        ret = ENOMEM;
>+        goto done;
We should immediately fail, if tmp_ctx could not be created.
Next two lines of code are usually used.
          DEBUG(SSSDBG_OP_FAILURE, ("talloc_new failed.\n"));
          return ENOMEM;

>+    }
> 
>     ret = sysdb_attrs_get_string(attrs, IPA_CN, &name);
>     if (ret != EOK) {
>-- 
>1.8.2.1
>

>From a6cf1dbee6a5ebf31936a47d6d96804f74db282d Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Sun, 19 May 2013 21:22:09 +0200
>Subject: [PATCH 2/4] Remove unneeded comment
>
>---
> src/providers/ldap/ldap_id.c | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
>index eb43f8d096a4009c2416988b261eb3089a863696..72fdd9c260383913e1fe24bf4daa283415840d0e 100644
>--- a/src/providers/ldap/ldap_id.c
>+++ b/src/providers/ldap/ldap_id.c
>@@ -1067,7 +1067,6 @@ void sdap_handle_account_info(struct be_req *breq, struct sdap_id_ctx *ctx)
>         req = groups_by_user_send(breq, be_ctx->ev, ctx,
>                                   ar->filter_value);
>         if (!req) ret = ENOMEM;
>-        /* tevent_req_set_callback(req, groups_by_user_done, breq); */
    Line was commented:  2009-06-13 12:17:55
    ACK
> 
>         tevent_req_set_callback(req, sdap_account_info_initgr_done, breq);
> 
>-- 
>1.8.2.1
>

>From db15a991b3c8740d10120cd873f76c1efd3fa63d Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Fri, 17 May 2013 21:51:09 -0400
>Subject: [PATCH 3/4] FO: Fix setting status of duplicates
>
>---
> src/providers/fail_over.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
>diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
>index c30ead2b3da4b7901807d21eeed96822282935e3..cd36327d8bbcd940389fd41315be71c3aada15e3 100644
>--- a/src/providers/fail_over.c
>+++ b/src/providers/fail_over.c
>@@ -658,6 +658,17 @@ static bool fo_server_match(struct fo_server *server,
>     return false;
> }
> 
>+static bool fo_server_cmp(struct fo_server *s1, struct fo_server *s2)
>+{
>+    char *name = NULL;
>+
>+    if (s2->common != NULL) {
          this function is called with s1<-siter, s2<-server
          so you check server->common != NULL,
          but before this patch siter->common was checked.
>+        name = s2->common->name;
>+    }
>+
>+    return fo_server_match(s1, name, s2->port, s2->user_data);
>+}
>+
> static bool fo_server_exists(struct fo_server *list,
>                              const char *name,
>                              int port,
>@@ -1371,14 +1382,11 @@ fo_set_port_status(struct fo_server *server, enum port_status status)
>      * into fo_server structures. Find the duplicates and set the same
>      * status */
>     DLIST_FOR_EACH(siter, server->service->server_list) {
>-        if (siter == server) continue;
>-        if (!siter->common || !siter->common->name) continue;
>-
>-        if (siter->port == server->port &&
>-            (strcasecmp(siter->common->name, server->common->name) == 0)) {
>-            DEBUG(7, ("Marking port %d of duplicate server '%s' as '%s'\n",
>-                      siter->port, SERVER_NAME(siter),
>-                      str_port_status(status)));
>+        if (fo_server_cmp(siter, server)) {
>+            DEBUG(SSSDBG_TRACE_FUNC,
>+                  ("Marking port %d of duplicate server '%s' as '%s'\n",
>+                   siter->port, SERVER_NAME(siter),
>+                   str_port_status(status)));
>             siter->port_status = status;
>             gettimeofday(&siter->last_status_change, NULL);
>         }
>-- 
>1.8.2.1
>

>From fae5ac11bc7c48684625b5b43d9c5856b11fa9c4 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Fri, 17 May 2013 14:33:26 +0200
>Subject: [PATCH 4/4] AD dyndns: extract the host name from URI
>
>---
> src/providers/ad/ad_dyndns.c | 31 ++++++++++++++++++++++---------
> 1 file changed, 22 insertions(+), 9 deletions(-)
>
>diff --git a/src/providers/ad/ad_dyndns.c b/src/providers/ad/ad_dyndns.c
>index 4e9ed852f91b8f99a0363b0f31af857ef98e8130..d6f2737e99b87b21f8ad6e5e1a8f008c68ee3698 100644
>--- a/src/providers/ad/ad_dyndns.c
>+++ b/src/providers/ad/ad_dyndns.c
>@@ -159,6 +159,7 @@ static void ad_dyndns_nsupdate_done(struct tevent_req *req)
> 
> struct ad_dyndns_update_state {
>     struct ad_options *ad_ctx;
>+    const char *servername;
> };
> 
> static void ad_dyndns_sdap_update_done(struct tevent_req *subreq);
>@@ -170,7 +171,7 @@ ad_dyndns_update_send(struct ad_options *ctx)
>     struct ad_dyndns_update_state *state;
>     struct tevent_req *req, *subreq;
>     struct sdap_id_ctx *sdap_ctx = ctx->id_ctx->sdap_id_ctx;
>-    const char *servername;
>+    LDAPURLDesc *lud;
> 
>     DEBUG(SSSDBG_TRACE_FUNC, ("Performing update\n"));
> 
>@@ -190,15 +191,27 @@ ad_dyndns_update_send(struct ad_options *ctx)
>     }
>     state->ad_ctx->dyndns_ctx->last_refresh = time(NULL);
> 
>-    if (strncmp(ctx->service->sdap->uri,
>-                "ldap://", 7) != 0) {
>-        DEBUG(SSSDBG_CRIT_FAILURE, ("Unexpected format of LDAP URI.\n"));
>-        ret = EIO;
>+    ret = ldap_url_parse(ctx->service->sdap->uri, &lud);
>+    if (ret != LDAP_SUCCESS) {
>+        DEBUG(SSSDBG_CRIT_FAILURE,
>+              ("Failed to parse ldap URI (%s)!\n", ctx->service->sdap->uri));
>+        ret = EINVAL;
>         goto done;
>     }
>-    servername = ctx->service->sdap->uri + 7;
>-    if (!servername) {
>-        ret = EIO;
>+
>+    if (lud->lud_host == NULL) {
>+        DEBUG(SSSDBG_CRIT_FAILURE,
>+              ("The LDAP URI (%s) did not contain a host name\n",
>+              ctx->service->sdap->uri));
>+        ldap_free_urldesc(lud);
>+        ret = EINVAL;
>+        goto done;
>+    }
>+
>+    state->servername = talloc_strdup(state, lud->lud_host);
>+    ldap_free_urldesc(lud);
>+    if (!state->servername) {
>+        ret = ENOMEM;
>         goto done;
>     }
> 
>@@ -214,7 +227,7 @@ ad_dyndns_update_send(struct ad_options *ctx)
>                                      NULL,
>                                      dp_opt_get_string(ctx->basic,
>                                                        AD_KRB5_REALM),
>-                                     servername,
>+                                     state->servername,
>                                      dp_opt_get_int(ctx->dyndns_ctx->opts,
>                                                     DP_OPT_DYNDNS_TTL),
>                                      false);
>-- 
>1.8.2.1
>

I did not test this patch, but it compiles without warnings and it looks good.

>_______________________________________________
>sssd-devel mailing list
>sssd-devel at lists.fedorahosted.org
>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list