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

Sumit Bose sbose at redhat.com
Tue May 28 08:26:54 UTC 2013


On Mon, May 27, 2013 at 08:37:08PM +0200, 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.

ACK

> 
> [PATCH 2/4] Remove unneeded comment
> Not needed anymore I think

ACK

> 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) {
> +        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)) {

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().

> +            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) {

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 .

bye,
Sumit
> +        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
> 



More information about the sssd-devel mailing list