Re: Review D38047 ... and then there was one....

From: Cy Schubert <Cy.Schubert_at_cschubert.com>
Date: Mon, 07 Oct 2024 15:08:30 UTC
In message <ZwPrDMYHxr9dOUPv@nuc>, Mark Johnston writes:
> On Sun, Oct 06, 2024 at 02:35:18PM -0400, David E. Cross wrote:
> > As I have been prodding about open reviews, there is now only one that
> > hasn't had any action.  One that is complete and in main (OMGYES), and one
> > that is at the finish line (I think).
> > 
> > 
> > That leaves just D38047 ( https://reviews.freebsd.org/D38047 ) ... Submitte
> d
> > Jan 13, **2023**, had initial peer review requests, I made all requested
> > changes (or explained why I didn't)  Made my last substantive changes Aug 4
> ,
> > **2023**, I have since then semi-regularly updated with fresh rebases, but
> > no other changes.
> > 
> > 
> > Please, love to get some eyes on this.  As it stands nscd is completely
> > useless for LDAP for getgroupmembership (and really ANY implementation that
> > defines a specific implementation of getgroupmembership, since it will then
> > bypass the non-existent NSCD version).  Additionally it fixes bugs with
> > negative caching as well as increases thread safety.
>
> One problem with this patch is that it fixes several bugs at once.
> Though the process can be a bit tedious, it's much easier to review
> smaller, self-contained patches which fix a single problem.  The change
> to the nscd Makefile plus the cycle prevention symbol rename, for
> instance, can be committed on its own.

Yes. I was about to suggest this. Plus, any proposed commit log message 
must answer the questions why, what and how. With special attention to why.

>
> I left some comments on the review in any case.  If you could split the
> patch into several smaller ones, it'd be greatly appreciated.
>
> > I've been running this successfully with LDAP for years now.
>


-- 
Cheers,
Cy Schubert <Cy.Schubert@cschubert.com>
FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy@nwtime.org>    Web:  https://nwtime.org

			e^(i*pi)+1=0