git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Derrick Stolee <derrickstolee@github.com>
Date
Mar 28, 2023, 18:32 UTC
Message-ID
<ca1f77db-a483-8b32-8e34-9bce1ea8afc3@github.com>
In-Reply-To
<xmqqfs9og47y.fsf@gitster.g>
On 3/28/23 1:45 PM, Junio C Hamano wrote:
Show 39 quoted lines
> Patrick Steinhardt <ps@pks.im> 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@pks.im>
>> ---
>>
>> 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

Previous: Junio C HamanoNext: Taylor Blau
Message 3 of 4 in “commit-graph: fix truncated generation numbers”
  1. commit-graph: fix truncated generation numbersPatrick Steinhardt, Mar 27, 2023
  2. Junio C HamanoMar 28, 2023
  3. Derrick StoleeMar 28, 2023
  4. Taylor BlauMar 28, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.