From a6b8f0143101f35bd5cc59ec5d51c9c4d428620c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:25 +0000 Subject: [PATCH 01/12] diffcore-break: guard against NULLed queue entries in merge loop The outer loop in `diffcore_merge_broken()` sets `q->queue[j]` to NULL when it merges a broken pair back together, and has a NULL check to skip such entries on subsequent iterations. The inner loop, however, lacks this guard: when it scans forward looking for a matching peer, it can encounter a slot that was NULLed by a previous outer-loop iteration and dereference it unconditionally. In practice this requires at least two broken pairs whose peers both survive rename/copy detection and appear later in the queue, which is rare but not impossible. Add the same `if (!pp) continue` guard to the inner loop. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- diffcore-break.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/diffcore-break.c b/diffcore-break.c index 17b5ad1fed..b5bcc956cc 100644 --- a/diffcore-break.c +++ b/diffcore-break.c @@ -289,6 +289,8 @@ void diffcore_merge_broken(void) */ for (j = i + 1; j < q->nr; j++) { struct diff_filepair *pp = q->queue[j]; + if (!pp) + continue; if (pp->broken_pair && !strcmp(pp->one->path, pp->two->path) && !strcmp(p->one->path, pp->two->path)) { From d07d09ee43b920f17d00a96ee29621e398136218 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:26 +0000 Subject: [PATCH 02/12] diff: handle NULL return from repo_get_commit_tree() The `repo_get_commit_tree()` function can return NULL when a commit's tree object is not available (e.g., the commit was parsed but its maybe_tree field is unset and the commit is not in the commit-graph). In cmd_diff(), the return value is immediately dereferenced via ->object without a NULL check, which would crash if the tree cannot be loaded. Add an explicit NULL check and die with a descriptive message. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/diff.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/builtin/diff.c b/builtin/diff.c index 4b46e394ce..18b1083e98 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -579,9 +579,13 @@ int cmd_diff(int argc, obj = deref_tag(the_repository, obj, NULL, 0); if (!obj) die(_("invalid object '%s' given."), name); - if (obj->type == OBJ_COMMIT) - obj = &repo_get_commit_tree(the_repository, - ((struct commit *)obj))->object; + if (obj->type == OBJ_COMMIT) { + struct tree *tree = repo_get_commit_tree( + the_repository, (struct commit *)obj); + if (!tree) + die(_("unable to read tree object for commit '%s'"), name); + obj = &tree->object; + } if (obj->type == OBJ_TREE) { if (sdiff.skip && bitmap_get(sdiff.skip, i)) From 2fdcce115a5ce4fe3d025858f958b92ab1c4d20e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:27 +0000 Subject: [PATCH 03/12] remote: guard `remote_tracking()` against NULL remote The `remote_tracking()` function unconditionally dereferences `remote->fetch` without checking whether remote is NULL. In practice, this never happens because the only caller (`apply_cas()`) guards the calls to this function by checking the `use_tracking` and `use_tracking_for_rest` attributes. However, it requires quite involved reasoning to reach that conclusion, and is therefore fragile. Just return -1 ("no tracking ref") when there is no remote to work with. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- remote.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/remote.c b/remote.c index e6c52c850c..b17648d6ef 100644 --- a/remote.c +++ b/remote.c @@ -2713,6 +2713,8 @@ static int remote_tracking(struct remote *remote, const char *refname, { char *dst; + if (!remote) + BUG("remote_tracking() called with NULL remote"); dst = apply_refspecs(&remote->fetch, refname); if (!dst) return -1; /* no tracking ref for refname at remote */ From d321f42c09b4f01978592cb2a4b8dc5fd86a5e12 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:28 +0000 Subject: [PATCH 04/12] reftable/stack: guard against NULL list_file in stack_destroy When reftable_new_stack() fails partway through initialization (e.g., reftable_buf_addstr returns an OOM error before reftable_buf_detach assigns p->list_file), it jumps to the error path which calls reftable_stack_destroy(p). At that point, p->list_file is still NULL because the detach never happened. reftable_stack_destroy() passes st->list_file unconditionally to read_lines(), which calls open(filename, O_RDONLY). Passing NULL to open() is undefined behavior and will typically crash. Guard the read_lines() call with a NULL check on st->list_file. When list_file is NULL, there are no table files to clean up anyway, so skipping read_lines is the correct behavior. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- reftable/stack.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/reftable/stack.c b/reftable/stack.c index ab12926708..c052b211fa 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -171,7 +171,8 @@ void reftable_stack_destroy(struct reftable_stack *st) st->merged = NULL; } - err = read_lines(st->list_file, &names); + if (st->list_file) + err = read_lines(st->list_file, &names); if (err < 0) { REFTABLE_FREE_AND_NULL(names); } From 775f1f2b19e1c3d133356170373fedeb363988a3 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:29 +0000 Subject: [PATCH 05/12] mailsplit: move NULL check before first use of file handle The `split_mbox()` function calls fileno(f) to check whether the input is a terminal, but the NULL check for f (from `fopen()`) does not happen until later. When the file cannot be opened, f is NULL, and `fileno(NULL)` is undefined behavior, typically crashing with a segmentation fault. Move the NULL check above the `isatty()`/`fileno()` call so the error path is taken before any use of the potentially-NULL handle. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/mailsplit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c index 264df6259a..0993418e63 100644 --- a/builtin/mailsplit.c +++ b/builtin/mailsplit.c @@ -225,14 +225,14 @@ static int split_mbox(const char *file, const char *dir, int allow_bare, FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r"); int file_done = 0; - if (isatty(fileno(f))) - warning(_("reading patches from stdin/tty...")); - if (!f) { error_errno("cannot open mbox %s", file); goto out; } + if (isatty(fileno(f))) + warning(_("reading patches from stdin/tty...")); + do { peek = fgetc(f); if (peek == EOF) { From 74fb18824cda5f28811e5d5e4653434f01238133 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:30 +0000 Subject: [PATCH 06/12] bisect: handle NULL commit in `bisect_successful()` When `lookup_commit_reference_by_name()` is called to find the first bad commit, the result is passed to `repo_format_commit_message()` immediately, which dereferences commit without checking for NULL. However, the commit could be NULL, even though in practice this is unlikely because `bisect_successful()` is only called after a successful bisect run has identified the bad commit, but the ref could still become dangling due to a concurrent gc or repository corruption. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/bisect.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/builtin/bisect.c b/builtin/bisect.c index 798e28f501..2672cbe5d1 100644 --- a/builtin/bisect.c +++ b/builtin/bisect.c @@ -663,6 +663,11 @@ static int bisect_successful(struct bisect_terms *terms) refs_read_ref(get_main_ref_store(the_repository), bad_ref, &oid); commit = lookup_commit_reference_by_name(bad_ref); + if (!commit) { + error(_("could not find commit for '%s'"), bad_ref); + free(bad_ref); + return BISECT_FAILED; + } repo_format_commit_message(the_repository, commit, "%s", &commit_name, &pp); From e87d701dc69a6b194f223f2405102cae4c4af03b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:31 +0000 Subject: [PATCH 07/12] replay: die when --onto does not peel to a commit The `peel_committish()` function calls `repo_peel_to_type()` to convert the given object to a commit, but does not check the return value. When the object exists but cannot be peeled to a commit (e.g., a tree or blob OID is passed as --onto), the return value is NULL. Add an explicit NULL check and die with a descriptive message in that case. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- replay.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/replay.c b/replay.c index da531d5bc6..b38cd5efe4 100644 --- a/replay.c +++ b/replay.c @@ -36,12 +36,16 @@ static struct commit *peel_committish(struct repository *repo, { struct object *obj; struct object_id oid; + struct commit *commit; if (repo_get_oid(repo, name, &oid)) die(_("'%s' is not a valid commit-ish for %s"), name, mode); obj = parse_object_or_die(repo, &oid, name); - return (struct commit *)repo_peel_to_type(repo, name, 0, obj, - OBJ_COMMIT); + commit = (struct commit *)repo_peel_to_type(repo, name, 0, obj, + OBJ_COMMIT); + if (!commit) + die(_("'%s' does not point to a commit for %s"), name, mode); + return commit; } static char *get_author(const char *message) From f5887e1ec0c89e1cc60dfbfb229a11e72e616cda Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:32 +0000 Subject: [PATCH 08/12] 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 Signed-off-by: Junio C Hamano --- revision.c | 9 +++++++-- t/t0410-partial-clone.sh | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/revision.c b/revision.c index 0c95edef59..693ee491b3 100644 --- a/revision.c +++ b/revision.c @@ -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) diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh index dff442da20..cc070019be 100755 --- a/t/t0410-partial-clone.sh +++ b/t/t0410-partial-clone.sh @@ -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 && From 436ef7c1e4131c31ae1a9152261c2c67c7affa21 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:33 +0000 Subject: [PATCH 09/12] pack-bitmap: handle missing bitmap for base MIDX When `prepare_midx_bitmap_git()` is called to load the bitmap for a chained MIDX's base layer, if the base MIDX does not have an associated bitmap file (e.g., it was not generated, or was deleted by gc), the return value is NULL. It is then stored in `bitmap_git->base` and immediately dereferenced on the next line. This can happen in practice with incremental MIDX chains: the base MIDX may have been written without `--write-bitmap-index`, or the bitmap may have been pruned while the incremental layer's bitmap still references it. Check the return value and go to the cleanup label (which unmaps the current bitmap and returns -1) so the caller falls back to non-bitmap object enumeration, matching the handling of other bitmap loading failures in the same function. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- pack-bitmap.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pack-bitmap.c b/pack-bitmap.c index 83eb47a28b..0552c5fbec 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -523,6 +523,10 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git, if (midx->base_midx) { bitmap_git->base = prepare_midx_bitmap_git(midx->base_midx); + if (!bitmap_git->base) { + warning(_("could not open bitmap for base MIDX")); + goto cleanup; + } bitmap_git->base_nr = bitmap_git->base->base_nr + 1; } else { bitmap_git->base_nr = 0; From 7384654acfa963512805ab0476602cbe8b0a2033 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:34 +0000 Subject: [PATCH 10/12] bisect: ensure non-NULL `head` before using it When `refs_resolve_ref_unsafe()` is called to resolve HEAD, and returns NULL (e.g., HEAD does not exist as a proper ref), the code falls back to `repo_get_oid("HEAD")` to try to resolve the OID directly. If that succeeds, execution continues with `head` still set to NULL. Later, that variable is passed to `repo_get_oid()` and `starts_with()`, both of which would dereference the NULL pointer. A concrete trigger for `refs_resolve_ref_unsafe()` returning NULL while `repo_get_oid()` succeeds could not be constructed against the ref backends currently in the tree; the naive case (a symbolic HEAD pointing at a nonexistent branch, in either the files or the reftable backend) fails in both calls consistently and returns via the existing `error(_("bad HEAD - I need a HEAD"))` path. Coverity, however, flags the leftover use of `head` after the outer `if (!head)` on a formal reading: `head` is still NULL at that point, and both `starts_with(head, ...)` and the second `repo_get_oid(..., head, ...)` in the else-branch would dereference it if that state were ever reached. Removing the outer check would risk regressing to a crash if a future ref backend ever manages to hit the "returns NULL for HEAD but has a valid OID for HEAD" state. Assigning the literal string "HEAD" as a safe fallback documents the intent and satisfies the analyzer without changing behavior in any code path we can currently reach. Assisted-by: Claude Opus 4.7 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/bisect.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/bisect.c b/builtin/bisect.c index 2672cbe5d1..3264e2da54 100644 --- a/builtin/bisect.c +++ b/builtin/bisect.c @@ -811,9 +811,11 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc, */ head = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", 0, &head_oid, &flags); - if (!head) + if (!head) { if (repo_get_oid(the_repository, "HEAD", &head_oid)) return error(_("bad HEAD - I need a HEAD")); + head = "HEAD"; + } /* * Check if we are bisecting From 83e717ee38e5c0ba43cc55762012a0c5da2a84f6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:35 +0000 Subject: [PATCH 11/12] shallow: fix NULL dereference After `write_one_shallow()` calls `lookup_commit()` to find the commit object for a shallow graft entry, it then checks `if (!c || ...)`. Inside that block, when the VERBOSE flag is set, it prints the OID being removed, via `c->object.oid`. But `c` can be NULL (the first condition in the `||` check). This happens when a shallow graft entry references a commit object that is not in the object store (e.g., after a partial fetch or in a corrupted repository). In that case, `lookup_commit()` returns NULL because the object cannot be found, the SEEN_ONLY check correctly decides to remove this entry from .git/shallow, but the verbose message crashes before the removal can complete. Use `graft->oid` instead of `c->object.oid` for the message. The graft entry's OID is the same value (it was used as the lookup key) and is always available regardless of whether the commit object exists. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- shallow.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shallow.c b/shallow.c index 07cae44ae5..2f96db5170 100644 --- a/shallow.c +++ b/shallow.c @@ -370,8 +370,7 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data) struct commit *c = lookup_commit(the_repository, &graft->oid); if (!c || !(c->object.flags & SEEN)) { if (data->flags & VERBOSE) - printf("Removing %s from .git/shallow\n", - oid_to_hex(&c->object.oid)); + printf("Removing %s from .git/shallow\n", hex); return 0; } } From 764255357846d79f9f960cd0bf0cdbbe21ed0f72 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 11:39:36 +0000 Subject: [PATCH 12/12] shallow: give write_one_shallow() its own hex buffer The previous fix reuses the local `hex` variable that is already computed at the top of `write_one_shallow()`. That works today, but `oid_to_hex()` returns a pointer into a small rotating buffer, so it is not stable across an unrelated call to `oid_to_hex()` from the same thread. A future edit that adds such a call between the assignment and the last user of `hex` would silently corrupt the output. Move `write_one_shallow()` off the rotating buffer entirely by using a local buffer instead. The current users of that `hex` variable are unchanged. Suggested-by: Junio C Hamano Assisted-by: Claude Opus 4.7 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- shallow.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/shallow.c b/shallow.c index 2f96db5170..c567cc3c69 100644 --- a/shallow.c +++ b/shallow.c @@ -359,7 +359,9 @@ struct write_shallow_data { static int write_one_shallow(const struct commit_graft *graft, void *cb_data) { 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; if (data->flags & QUICK) {