Subject: Re: [PATCH 3/6] commit-graph: compute generation numbers

From: Jonathan Tan <[email-masked]>
Date: Tue, 3 Apr 2018 11:30:10 -0700

On Tue,  3 Apr 2018 12:51:40 -0400
Derrick Stolee <[email-masked]> wrote:

> +		if ((*list)->generation != GENERATION_NUMBER_UNDEF) {
> +			if ((*list)->generation > GENERATION_NUMBER_MAX)
> +				die("generation number %u is too large to store in commit-graph",
> +				    (*list)->generation);
> +			packedDate[0] |= htonl((*list)->generation << 2);
> +		}

The die() should have "BUG:" if you agree with my comment below.

> +static void compute_generation_numbers(struct commit** commits,
> +				       int nr_commits)

Style: space before **, not after.

> +			if (all_parents_computed) {
> +				current->generation = max_generation + 1;
> +				pop_commit(&list);
> +			}

I think the current->generation should be clamped to _MAX here. If we do, then
the die() I mentioned in my first comment will have "BUG:", since we are never
meant to write any number larger than _MAX in ->generation.