On Wed, Apr 12, 2023 at 01:56:58PM -0400, Taylor Blau wrote:
> On Wed, Apr 12, 2023 at 12:22:31PM +0200, Patrick Steinhardt wrote:
> > Fix this bug by exiting early in case we have determined that the MIDX
> > wouldn't have any packfiles to index. While the request itself does not
> > make much sense anyway, it is still preferable to exit gracefully than
> > to abort.
>
> Interesting. This reminded me quite a bit of eb57277ba3 (midx: prevent
> writing a .bitmap without any objects, 2022-02-09) which tackled a
> similar problem of trying to write a MIDX bitmap without any objects.
>
> We may want to consider moving that conditional further up, since this
> makes the conditional added in eb57277ba3 dead code AFAICT. Here's a
> patch on top of this one that I think would do the trick.
>
> It has the added benefit of sticking a:
>
> warning: unknown preferred pack: 'does-not-exist'
>
> in the output before dying, which might be nice (though I doubt anybody
> will ever see it ;-)). The main difference is that we unset the bitmap
> related bits from `flags`, which avoids us trying to compute a preferred
> pack in the first place.
>
> For it to work, though, we need to make sure that ctx.preferred_pack_idx
> is set to -1, and not zero-initialized, since we'll segfault otherwise
> when trying to read into an empty array.
Indeed, that is a good point. I think we can simplify your patch even
further in that case:
diff --git a/midx.c b/midx.c
index 47989f7ea7..67eb617591 100644
--- a/midx.c
+++ b/midx.c
@@ -1328,17 +1328,17 @@ static int write_midx_internal(const char *object_dir,
}
if (preferred_pack_name) {
- int found = 0;
+ ctx.preferred_pack_idx = -1;
+
for (i = 0; i < ctx.nr; i++) {
if (!cmp_idx_or_pack_name(preferred_pack_name,
ctx.info[i].pack_name)) {
ctx.preferred_pack_idx = i;
- found = 1;
break;
}
}
- if (!found)
+ if (ctx.preferred_pack_idx == -1)
warning(_("unknown preferred pack: '%s'"),
preferred_pack_name);
} else if (ctx.nr &&
The other cases already set `preferred_pack_idx = -1`, so this is really
all we need to do to fix the segfault.
Patrick
Attachment:
signature.asc
Description: PGP signature
