On 4/13/2023 7:16 AM, Patrick Steinhardt wrote:
> Fix this bug by resetting the preferred packfile index to `-1` before
> searching for the preferred pack. This fixes the segfault as we already
> check for whether the index is `> - 1`. If it is not, we simply don't
> pick a preferred packfile at all.
> if (preferred_pack_name) {
> - int found = 0;
> + ctx.preferred_pack_idx = -1;
> +
This patch looks good, but I did need to think about it for a bit,
so here are my thoughts (feel free to ignore):
I briefly considered recommending that we set this as the default in
an initializer macro, something like
#define WRITE_MIDX_CONTEXT_INIT { .preferred_pack_idx = -1 }
but this struct is internal to the file and only constructed once
(at the start of write_midx_internal).
Further, outside the context of this patch we have this code:
} else if (ctx.nr &&
(flags & (MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP))) {
struct packed_git *oldest = ctx.info[ctx.preferred_pack_idx].p;
ctx.preferred_pack_idx = 0;
I don't see a way that ctx.preferred_pack_idx can be anything but zero
here, but it's also not going to give a segfault because of the 'ctx.nr'
condition.
Thanks,
-Stolee
