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

Sumit Bose sbose at redhat.com
Tue May 28 09:14:33 UTC 2013


On Tue, May 28, 2013 at 10:56:06AM +0200, Jakub Hrozek wrote:
> 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?

yes

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

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

I was more thinking in terms of code reuse, since this part of the code
is pretty generic chances are that it get extracted and reused in other
places as well.

All my concerns are addressed, ACK.

bye,
Sumit



More information about the sssd-devel mailing list