Re: [PATCH 11/11] shallow: fix NULL dereference
From: Junio C Hamano <hidden>
Date: 2026-07-09 20:10:14
"Johannes Schindelin via GitGitGadget" [off-list ref] writes:
quoted hunk ↗ jump to hunk
diff --git a/shallow.c b/shallow.c index 07cae44ae5..3d2230351e 100644 --- a/shallow.c +++ b/shallow.c@@ -371,7 +371,7 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data) if (!c || !(c->object.flags & SEEN)) { if (data->flags & VERBOSE) printf("Removing %s from .git/shallow\n", - oid_to_hex(&c->object.oid)); + oid_to_hex(&graft->oid)); return 0;
Haha. We come into this block and emit this message when we may not
even have a valid 'c', yet we use c->object.oid there. It makes
perfect sense to use graft->oid here instead, as your patch does.
However, its hexadecimal representation has already been computed in
the local variable 'hex', and the "happy path" code after this
section seems to assume that 'hex' is still valid (even though
oid_to_hex() uses rotating 4-element buffer, which makes the
assumption a risky one).
We should use "hex" here instead of oid_to_hex(&graft->oid), which
does not add to the existing risk. In addition, if we add something
like:
struct write_shallow_data *data = cb_data;
- const char *hex = oid_to_hex(&graft->oid);
+ char hex[GIT_MAX_HEXSZ + 1];
+
+ oid_to_hex_r(hex, &graft->oid);
if (graft->nr_parent != -1)
return 0;
to the beginning of the function, we can get rid of existing
riskiness entirely.