Re: [PATCH v2 08/15] midx: allow marking a pack as preferred

From: Taylor Blau <me@ttaylorr.com>
Date: Mar 4, 2021, 03:04 UTC
Message-ID: <YEBOKpaD1gGXRTvp@nand.local>
In-Reply-To: <20210304020017.1802260-1-jonathantanmy@google.com>

On Wed, Mar 03, 2021 at 06:00:17PM -0800, Jonathan Tan wrote:

Show 17 quoted lines

> > I
> > think ultimately you want something like this squashed in:
> >
> > --- >8 ---
> >
> > diff --git a/midx.c b/midx.c
> > index d2c56c4bc6..46f55ff6cf 100644
> > --- a/midx.c
> > +++ b/midx.c
> > @@ -582,7 +582,7 @@ static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,
> >  						  struct pack_info *info,
> >  						  uint32_t nr_packs,
> >  						  uint32_t *nr_objects,
> > -						  uint32_t preferred_pack)
> > +						  int preferred_pack)
>
> Why this change?

This was wrong in the original patch: ctx.preferred_pack is an integer, and is set to -1 when no preferred pack was specified.

It's certainly unlikely that we'd have 2^31 packs, but silently converting a signed type to an unsigned one is misleading.

> The rest makes sense.

Thanks for taking a look.

Taylor