From df00334f8b8cb85a928e1ca22aa12dd6b87fb154 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 26 Apr 2026 12:44:29 +0200 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 --- diffcore-break.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/diffcore-break.c b/diffcore-break.c index 17b5ad1fedeb3b..b5bcc956ccb831 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 4fdba0542b3d643affe32ec35f27fdbabccf54d0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 26 Apr 2026 16:48:18 +0200 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 --- builtin/diff.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/builtin/diff.c b/builtin/diff.c index 4b46e394cecb8d..18b1083e984a35 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 1398a2f1200da5cbc716b1a926aa614ef6c13503 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 26 Apr 2026 16:51:16 +0200 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 --- remote.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/remote.c b/remote.c index 00723b385e1d52..887e388f9cac68 100644 --- a/remote.c +++ b/remote.c @@ -2681,6 +2681,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 285f019fb3c3f1d3eb6066de93315acf273dca69 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 26 Apr 2026 17:48:12 +0200 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 --- reftable/stack.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/reftable/stack.c b/reftable/stack.c index 1fba96ddb3366f..3fc3c0b2d1e9c4 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 12c2c8450e0f0ee71cc6beefe6186e0ededdf806 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 28 Apr 2026 14:30:54 +0200 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 --- builtin/mailsplit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c index 264df6259a0411..0993418e630dec 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 ca818ee405a8078e14ec4faab2422b34c6e83681 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 28 Apr 2026 20:03:31 +0200 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 --- builtin/bisect.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/builtin/bisect.c b/builtin/bisect.c index e7c2d2f3bb0f4a..408e0f414e2f50 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 8216769be9eec7489f1039e0211f3e6d3388247b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 29 Apr 2026 08:25:38 +0200 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 --- replay.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/replay.c b/replay.c index da531d5bc68add..b38cd5efe49c50 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 41285dd8e1df9d010648459d5ff93db72bff7c1a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 29 Apr 2026 09:12:35 +0200 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 --- 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 e91d7e1f11356a..7f3999b5510b07 100644 --- a/revision.c +++ b/revision.c @@ -1903,8 +1903,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 dff442da2090b5..cc070019bee9fa 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 cccd36137f49e7523f8ee15405574943536cb505 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 29 Apr 2026 11:55:29 +0200 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 --- pack-bitmap.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pack-bitmap.c b/pack-bitmap.c index e8a82945cc319e..ca7998c10b4876 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 376a6581cbdc3c5df624658f19cf19aac7694bc7 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 30 Apr 2026 10:06:44 +0200 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 --- builtin/bisect.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/bisect.c b/builtin/bisect.c index 408e0f414e2f50..dccf0be6bbb1d8 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 e581bc91ee41731a79c1843f648a11e969c2e16a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 5 May 2026 00:33:17 +0200 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 --- shallow.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shallow.c b/shallow.c index 07cae44ae52a04..2f96db5170bcc8 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 2ef74b52fadc2bbffe7414b27db739546ab369c1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 Jul 2026 09:26:07 +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 --- shallow.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/shallow.c b/shallow.c index 2f96db5170bcc8..c567cc3c69fe4c 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) {