On 3/28/23 1:45 PM, Junio C Hamano wrote: > Patrick Steinhardt <ps@xxxxxx> 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 <ps@xxxxxx> >> --- >> >> 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? Yes, looks good. What a silly mistake, but thanks for going the extra mile to introduce a test that will prevent it in the future. Thanks, -Stolee
