Re: [PATCH v9] show-branch: convert per-branch flags to commit-slab
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 21, 2026, 18:51 UTC
- Message-ID
- <xmqqzezkw4zq.fsf@gitster.g>
- In-Reply-To
- <20260717224435.74504-1-gatlavishweshwarreddy26@gmail.com>
Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:
> I took time to read through the code carefully before sending this.
Hmph. I hate to say this, but I am finding it difficult to trust your "carefully" at this point.
$ make
$ ./git show-branch master next
Floating point exception (core dumped).While I have not spent the time to exhaustively find all bugs in this code [*], it is disturbing that the simplest use of the command immediately crashes. The reason for this crash is trivial.
Show 10 quoted lines
> static struct commit_name_slab name_slab;
>
> +define_commit_slab(commit_rev_flags, uint64_t);
> +static struct commit_rev_flags rev_flags_slab;
> +static int flags_stride; /* number of uint64_t words per commit */
> +
> static struct commit_name *commit_to_name(struct commit *commit)
> {
> return *commit_name_slab_at(&name_slab, commit);
> }The code still uses name_slab to associate names with commits, and commit_to_name() is called by functions like name_commits(), which is used in cmd_show_branch(). Yet, the patch does this:
Show 6 quoted lines
> @@ -713,8 +773,6 @@ int cmd_show_branch(int ac, > const char **args_copy = NULL; > int ret; > > - init_commit_name_slab(&name_slab); > -
The code simply discards the initialization for that slab, leaving name_slab BSS-initialized. Consequently, all members in the struct are initialized to 0, including the '.slab_size' member.
This init_commit_name_slab() call is not moved elsewhere; it simply disappeared without an explanation.
When commit_to_name() tries to look up a commit in the slab, it first attempts to determine which slab should contain the data for the commit, using this code from <commit-slab-impl.h>:
scope elemtype *slabname## _at_peek(struct slabname *s, \
const struct commit *c, \
int add_if_missing) \
{ \
unsigned int nth_slab, nth_slot; \
\
nth_slab = c->index / s->slab_size; \
nth_slot = c->index % s->slab_size; \
\
And we all know what would happen when you divide by zero.[Footnote]
* ... and I will not.