On (20/09/13 10:54), Pavel Březina wrote: >On 09/20/2013 02:30 AM, Simo Sorce wrote: >>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. > >I agree. We should not introduce new labels. I agree too. Compilers are smart enough to do this kind of optimization.
LS _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
