revision: avoid dereferencing NULL in `add_parents_only()`
This function resolves revision suffixes like commit^@ (all parents),
commit^! (commit minus parents), and commit^-N (exclude Nth parent). It
calls `get_reference()` in a loop to peel through tag objects until it
reaches a commit.
The existing NULL check after `get_reference()` only handles the
ignore_missing case, but get_reference() can return NULL through three
distinct paths:
1. revs->ignore_missing: the caller asked to silently skip missing
objects.
2. revs->exclude_promisor_objects: the object is a lazy promisor
object that should be excluded from the walk.
3. revs->do_not_die_on_missing_objects: the caller wants to record
missing OIDs for later reporting (used by `git rev-list
--missing=print`) rather than dying.
In the latter two instances, the code falls through to dereference the
NULL pointer.
Handle all three cases explicitly:
- ignore_missing: return 0, matching the existing behavior and
the pattern in `handle_revision_arg()`.
- do_not_die_on_missing_objects: return 0. The missing OID has already
been recorded in `revs->missing_commits` by `get_reference()`.
Returning 0 is consistent with `handle_revision_arg()` and
`process_parents()`, both of which continue without error when this flag
is set. The broader codebase pattern for this flag is "record and
continue": list-objects.c, builtin/rev-list.c, and process_parents
all skip the die/error and keep walking.
- everything else (only the `exclude_promisor_objects` case in
practice): return -1, consistent with `handle_revision_arg()` where
the condition only matches `ignore_missing` or
`do_not_die_on_missing_objects`, falling through to ret = -1 for the
promisor case.
Note: the callers of `add_parents_only()` in
`handle_revision_pseudo_opt()` treat any nonzero return as "handled"
(`if (add_parents_only(...)) { ret = 0; }`), so the -1 for the promisor
case is indistinguishable from success there. This means a
promisor-excluded tag target referenced via commit^@ would be silently
skipped rather than producing an error. This is a pre-existing
limitation of the caller's return value handling and not made worse by
this change; the alternative (a NULL dereference crash) _would be_
strictly worse.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
main
parent
e87d701dc6
commit
f5887e1ec0
|
|
@ -1904,8 +1904,13 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,
|
|||
return 0;
|
||||
while (1) {
|
||||
it = get_reference(revs, arg, &oid, 0);
|
||||
if (!it && revs->ignore_missing)
|
||||
return 0;
|
||||
if (!it) {
|
||||
if (revs->ignore_missing)
|
||||
return 0;
|
||||
if (revs->do_not_die_on_missing_objects)
|
||||
return 0;
|
||||
return -1;
|
||||
}
|
||||
if (it->type != OBJ_TAG)
|
||||
break;
|
||||
if (!((struct tag*)it)->tagged)
|
||||
|
|
|
|||
|
|
@ -489,6 +489,24 @@ test_expect_success 'rev-list dies for missing objects on cmd line' '
|
|||
done
|
||||
'
|
||||
|
||||
test_expect_success '--exclude-promisor-objects with ^@ on missing object' '
|
||||
rm -rf repo &&
|
||||
test_create_repo repo &&
|
||||
test_commit -C repo foo &&
|
||||
test_commit -C repo bar &&
|
||||
|
||||
COMMIT=$(git -C repo rev-parse foo) &&
|
||||
promise_and_delete "$COMMIT" &&
|
||||
|
||||
git -C repo config core.repositoryformatversion 1 &&
|
||||
git -C repo config extensions.partialclone "arbitrary string" &&
|
||||
|
||||
# Ensure that "$COMMIT^@" is handled gracefully even though the
|
||||
# actual commits are missing.
|
||||
git -C repo rev-list --exclude-promisor-objects "$COMMIT^@" >out &&
|
||||
test_must_be_empty out
|
||||
'
|
||||
|
||||
test_expect_success 'single promisor remote can be re-initialized gracefully' '
|
||||
# ensure one promisor is in the promisors list
|
||||
rm -rf repo &&
|
||||
|
|
|
|||
Loading…
Reference in New Issue