Re: [PATCH v5 3/5] refs: selectively set prefix in the seek functions

From: Jeff King <peff@peff.net>
Date: Jul 17, 2025, 21:55 UTC
Message-ID: <20250717215506.GA2244266@coredump.intra.peff.net>
In-Reply-To: <CAOLa=ZTdUsv-6jVhPSwEW5dYr3vEh2bLT5ECRZZ1ot3MGAYhBg@mail.gmail.com>

On Thu, Jul 17, 2025 at 12:49:33PM -0700, Karthik Nayak wrote:

Show 26 quoted lines

> > diff --git a/refs/ref-cache.c b/refs/ref-cache.c
> > index 1d95b56d40..3949d145e8 100644
> > --- a/refs/ref-cache.c
> > +++ b/refs/ref-cache.c
> > @@ -498,13 +498,14 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator,
> >  		 * indexing to each level as needed.
> >  		 */
> >  		do {
> > -			int len, idx;
> > +			size_t len;
> > +			int idx;
> >  			int cmp = 0;
> >
> >  			sort_ref_dir(dir);
> >
> >  			slash = strchr(slash, '/');
> > -			len = slash ? slash - refname : (int)strlen(refname);
> > +			len = slash ? slash - refname : strlen(refname);
> >
> >  			for (idx = 0; idx < dir->nr; idx++) {
> >  				cmp = strncmp(refname, dir->entries[idx]->name, len);
> >
> > -Peff
> 
> Thanks, I think we have to typecast `slash - refname` to size_t, but
> this is the right way to do it. Thanks for the review!

Ah, yeah. I mistakenly test-compiled without DEVELOPER=1. ;)

I do think that cast is a lesser evil, though. It is a ptrdiff_t, but we know it is correctly unsigned because "slash > refname" via strchr. I wish there was a good way to use the type system to tell the compiler that.

-Peff