Subject: Re: [PATCH] commit-graph: fix truncated generation numbers

From: Junio C Hamano <[email-masked]>
Date: Tue, 28 Mar 2023 10:45:05 -0700

Patrick Steinhardt <[email-masked]> writes:

> In 80c928d947 (commit-graph: simplify compute_generation_numbers(),
> 2023-03-20), the code to compute generation numbers was simplified to
> use the same infrastructure as is used to compute topological levels.
> This refactoring introduced a bug where the generation numbers are
> truncated when they exceed UINT32_MAX because we explicitly cast the
> computed generation number to `uint32_t`. This is not required though:
> both the computed value and the field of `struct commit_graph_data` are
> of the same type `timestamp_t` already, so casting to `uint32_t` will
> cause truncation.
>
> This cast can cause us to miscompute generation data overflows:
>
>     1. Given a commit with no parents and committer date
>        `UINT32_MAX + 1`.
>
>     2. We compute its generation number as `UINT32_MAX + 1`, but
>        truncate it to `1`.
>
>     3. We calculate the generation offset via `$generation - $date`,
>        which is thus `1 - (UINT32_MAX + 1)`. The computation underflows
>        and we thus end up with an offset that is bigger than the maximum
>        allowed offset.
>
> As a result, we'd be writing generation data overflow information into
> the commit-graph that is bogus and ultimately not even required.
>
> Fix this bug by removing the needless cast.
>
> Signed-off-by: Patrick Steinhardt <[email-masked]>
> ---
>
> This commit applies on top of cbfe360b14 (commit-reach: add
> tips_reachable_from_bases(), 2023-03-20), which has recently been merged
> to next.

The patch is clearly explained and the change looks quite
straight-forward.  Derrick, Ack?

Thanks.