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

Jakub Hrozek jhrozek at redhat.com
Tue May 28 08:56:06 UTC 2013


On Tue, May 28, 2013 at 10:26:54AM +0200, Sumit Bose wrote:
> > @@ -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)) {
> 
> You removed the NULL check on siter->common->name but siter is used in
> fo_server_cmp() as first argument to fo_server_match() where it is not
> properly checked if siter->common->name is not NULL.
> 
> I guess it's good to remove the check here and add it to
> fo_server_match().
> 

Sorry, I'm not sure I completely understood. I moved the
server->common->name check to fo_server_match(), is that what you meant?

> > 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) {
> 
> maybe you can check if strcasecmp( lud->lud_scheme, "ldapi" ) == 0
> because in this case lud->lud_host contains a local path and not a host
> name. Please note that lud->lud_scheme might be NULL .

You're right, I was thinking about the schemes, too, but wasn't sure if
there is a way to actually have an AD server with ldapi ? But it's
better to be safe.
-------------- next part --------------
>From a93c760ea90b40866c736d74163520bd3ca58606 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 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index 98fc69f14d62d74fb82b0f067307362e819aa82c..95a11198a99aa60e93a1f5af18c88ff918dcd53a 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -216,6 +216,9 @@ static errno_t ipa_subdom_store(struct sss_domain_info *domain,
     int ret;
 
     tmp_ctx = talloc_new(domain);
+    if (tmp_ctx == NULL) {
+        return ENOMEM;
+    }
 
     ret = sysdb_attrs_get_string(attrs, IPA_CN, &name);
     if (ret != EOK) {
-- 
1.8.2.1

-------------- next part --------------
>From 10d3e753aa63073420cf27f020a1095ac7e5dca7 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); */
 
         tevent_req_set_callback(req, sdap_account_info_initgr_done, breq);
 
-- 
1.8.2.1

-------------- next part --------------
>From 4a657fabce89ca773e57bf3c3522cf8b5b90e93a 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 | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index c30ead2b3da4b7901807d21eeed96822282935e3..81fc45ce3ca46018dfb9fcccea47bc434a9608d7 100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -650,7 +650,8 @@ static bool fo_server_match(struct fo_server *server,
         return true;
     }
 
-    if (name != NULL && server->common != NULL) {
+    if (name != NULL &&
+        server->common != NULL && server->common->name != NULL) {
         if (!strcasecmp(name, server->common->name))
             return true;
     }
@@ -658,6 +659,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) {
+        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 +1383,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

-------------- next part --------------
>From 407799ba13e3ba1cc5292f66f825e4631bc93b5b 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 | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/src/providers/ad/ad_dyndns.c b/src/providers/ad/ad_dyndns.c
index 4e9ed852f91b8f99a0363b0f31af857ef98e8130..da01426ca2e128b339297d2c6d23467337ab3eb0 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,36 @@ 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_scheme != NULL &&
+        strcasecmp(lud->lud_scheme, "ldapi") == 0) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("The LDAP scheme is ldapi://, cannot proceed with update\n"));
+        ldap_free_urldesc(lud);
+        ret = EINVAL;
+        goto done;
+    }
+
+    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 +236,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



More information about the sssd-devel mailing list