Re: [PATCH 3/7] pack-bitmap: allow aborting iteration of bitmapped objects

[Patrick Steinhardt <ps@pks.im>]

On Thu, Jul 09, 2026 at 03:19:52PM -0500, Justin Tobler wrote:
> > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> > index ea5eab4cf8..8ff92c5272 100644
> > --- a/builtin/pack-objects.c
> > +++ b/builtin/pack-objects.c
> > @@ -1909,7 +1909,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
> >  		return 0;
> >  
> >  	create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
> > -	return 1;
> > +	return 0;
> 
> I wonder why this was even returning 1 to begin with? As you mentioned,
> the return value appears to be ignored anyways. I'm assuming it was
> signal that an object entry was created?

The function is only called from a single location, and the return value
was completely ignored until this commit. It has always been this way
since the function was originally introduced in 6b8fda2db1
(pack-objects: use bitmaps when packing objects, 2013-12-21), so it
never seemed to have any purpose. The commit message doesn't mention
anything either.

> > diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> > index 8f63003709..02818b81c6 100644
> > --- a/builtin/rev-list.c
> > +++ b/builtin/rev-list.c
> > @@ -486,7 +486,7 @@ static int show_object_fast(
> >  	void *payload UNUSED)
> >  {
> >  	fprintf(stdout, "%s\n", oid_to_hex(oid));
> > -	return 1;
> > +	return 0;
> 
> Also curious about this one too. It probably doesn't matter though.

Likewise, this was introduced in aa32939fea (rev-list: add bitmap mode
to speed up object lists, 2013-12-21), and the return value wasn't ever
used for anything.

Patrick