[SSSD] [PATCH] NSS: Set UID and GID to negative cache after searching all domains

Simo Sorce simo at redhat.com
Fri Sep 20 00:30:08 UTC 2013


On Thu, 2013-09-19 at 18:45 +0200, Jakub Hrozek wrote:
> On Wed, Sep 18, 2013 at 10:40:13PM +0200, Jakub Hrozek wrote:
> > On Wed, Sep 18, 2013 at 01:41:36PM -0400, Simo Sorce wrote:
> > > On Wed, 2013-09-18 at 17:58 +0200, Jakub Hrozek wrote:
> > > > Hi,
> > > > 
> > > > the first patch fixes the bug that Jean-Baptiste Denis found earlier
> > > > today. In case the lookup was not done using a FQDN, the code would skip
> > > > setting the entries to the ncache.
> > > > 
> > > > The second patch is an incremental improvement. I don't think we should
> > > > abort the whole lookup if setting an entry in negcache would fail. The
> > > > negative cache is a performance optimization after all.
> > > 
> > > It seem to me that with the first patch you are changing behavior as you
> > > leave 'ret' unchanged to whatever error is returned instead of setting
> > > it to ENOENT before going to 'done'.
> > > 
> > > Simo.
> > 
> > Ugh, that is a bug.
> > 
> > I was going back and forth on changing the particular return to goto
> > and when I made my mind, I forgot to set the errno.
> > 
> > Thanks Simo for catching it, I'll prepare a new version.
> 
> I think we were both confused by the poor label naming. The label actually,
> despite being named "done", made the function return ENOENT so the code
> was correct. But even I was confused couple of hours after sending the
> patch so the code had to be improved :)
> 
> So in the attached patch I renamed the label to notfound. I hope that's
> OK if we don't use the commonly used "done" in this case. I think the most
> important thing is that there is only a single label we jump to.
> 
> Alternatively, if you prefer strictly one exit point, I could convert
> the function to only use goto done and set negcache based on errno value
> (if ENOENT->ncache) but I didn't see that as necessary.

I prefer the idiom:
  ret = ENOENT;
  goto done;

to:
  goto notfound;


Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list