1
0
mirror of https://github.com/git/git.git synced 2025-02-06 09:26:16 +00:00

run_diff_files(): de-mystify the size of combine_diff_path struct

We allocate a combine_diff_path struct with space for 5 parents. Why 5?

The history is not particularly enlightening. The allocation comes from
b4b1550315 (Don't instantiate structures with FAMs., 2006-06-18), which
just switched to xmalloc from a stack struct with 5 elements. That
struct changed to 5 from 4 in 2454c962fb (combine-diff: show mode
changes as well., 2006-02-06), when we also moved from storing raw sha1
bytes to the combine_diff_parent struct. But no explanation is given.
That 4 comes from the earliest code in ea726d02e9 (diff-files: -c and
--cc options., 2006-01-28).

One might guess it is for the 4 stages we can store in the index. But
this code path only ever diffs the current state against stages 2 and 3.
So we only need two slots.

And it's easy to see this is still the case. We fill the parent slots by
subtracting 2 from the ce_stage() values, ignoring values below 2. And
since ce_stage() is only 2 bits, there are 4 values, and thus we need 2
slots.

Let's use the correct value (saving a tiny bit of memory) and add a
comment explaining what's going on (saving a tiny bit of programmer
brain power).

Arguably we could use:

  1 + (STAGEMASK >> STAGESHIFT) - 2

which lets the compiler enforce that we will not go out-of-bounds if we
see an unexpected value from ce_stage(). But that is more confusing to
explain, and the constant "2" is baked into other parts of the function.
It is a fundamental constant, not something where somebody might bump a
macro and forget to update this code.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Jeff King 2025-01-09 03:44:21 -05:00 committed by Junio C Hamano
parent 30f7414ca1
commit ca3abe41d7

View File

@ -166,8 +166,13 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
wt_mode = 0;
}
/*
* Allocate space for two parents, which will come from
* index stages #2 and #3, if present. Below we'll fill
* these from (stage - 2).
*/
dpath = combine_diff_path_new(ce->name, ce_namelen(ce),
wt_mode, null_oid(), 5);
wt_mode, null_oid(), 2);
while (i < entries) {
struct cache_entry *nce = istate->cache[i];