Re: [PATCH 1/5] pack-bitmap.c: fix broken warning() when missing MIDX'd pack

From: Taylor Blau <me@ttaylorr.com>
Date: May 28, 2025, 02:00 UTC
Message-ID: <aDZuMciIqrP2aPq8@nand.local>
In-Reply-To: <aDQXBBrL2Ni69gwE@pks.im>

On Mon, May 26, 2025 at 09:23:48AM +0200, Patrick Steinhardt wrote:

Show 15 quoted lines

> On Sun, May 25, 2025 at 02:41:51PM -0400, Taylor Blau wrote:
> > diff --git a/pack-bitmap.c b/pack-bitmap.c
> > index b9f1d86604..99c4927e9c 100644
> > --- a/pack-bitmap.c
> > +++ b/pack-bitmap.c
> > @@ -490,7 +490,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
> >  	for (i = 0; i < bitmap_git->midx->num_packs + bitmap_git->midx->num_packs_in_base; i++) {
> >  		if (prepare_midx_pack(bitmap_repo(bitmap_git), bitmap_git->midx, i)) {
> >  			warning(_("could not open pack %s"),
> > -				bitmap_git->midx->pack_names[i]);
> > +				bitmap_git->midx->pack_names[i - bitmap_git->midx->num_packs_in_base]);
>
> Doesn't this cause a negative array index though in the case where
> `prepare_midx_pack()` returns an error for any `i` smaller than the
> number of packs in base?

Nice catch, yeah, this is definitely broken when the value i is smaller than m->num_packs_in_base.

This is another spot that would benefit from similar treatment where callers (outside of midx.c and *maybe* midx-write.c) access the pack_names array through a function liked nth_midxed_pack_name() or similar.

I'll adjust this and send a new round.

Thanks, Taylor