{"thread":{"id":"65960","subject":"[PATCH 00/11] coverity: avoid dereferencing NULL","startedAt":"2026-07-09T09:42:41Z","lastAt":"2026-07-10T15:46:43Z","messageCount":36,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"547584","messageId":"pull.2174.git.1783590159.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":null,"subject":"[PATCH 00/11] coverity: avoid dereferencing NULL","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:27Z","receivedAt":"2026-07-09T09:42:41Z","isPatch":true,"body":"This is a continuation of the effort I started in the patch series that\nbecame js/coverity-fixes. This next batch adds guards to avoid dereferencing\nNULL pointers and accessing NULL file descriptors.\n\nJohannes Schindelin (11):\n diffcore-break: guard against NULLed queue entries in merge loop\n diff: handle NULL return from repo_get_commit_tree()\n remote: guard `remote_tracking()` against NULL remote\n reftable/stack: guard against NULL list_file in stack_destroy\n mailsplit: move NULL check before first use of file handle\n bisect: handle NULL commit in `bisect_successful()`\n replay: die when --onto does not peel to a commit\n revision: avoid dereferencing NULL in `add_parents_only()`\n pack-bitmap: handle missing bitmap for base MIDX\n bisect: ensure non-NULL `head` before using it\n shallow: fix NULL dereference\n\n builtin/bisect.c | 9 ++++++++-\n builtin/diff.c | 10 +++++++---\n builtin/mailsplit.c | 6 +++---\n diffcore-break.c | 2 ++\n pack-bitmap.c | 4 ++++\n reftable/stack.c | 3 ++-\n remote.c | 2 ++\n replay.c | 8 ++++++--\n revision.c | 9 +++++++--\n shallow.c | 2 +-\n 10 files changed, 42 insertions(+), 13 deletions(-)\n\n\nbase-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2174%2Fdscho%2Fcoverity-fixes-null-safety-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2174/dscho/coverity-fixes-null-safety-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2174\n-- \ngitgitgadget\n"},{"id":"547585","messageId":"df00334f8b8cb85a928e1ca22aa12dd6b87fb154.1783590159.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH 01/11] diffcore-break: guard against NULLed queue entries in merge loop","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:28Z","receivedAt":"2026-07-09T09:42:42Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe outer loop in `diffcore_merge_broken()` sets `q->queue[j]` to NULL\nwhen it merges a broken pair back together, and has a NULL check to skip\nsuch entries on subsequent iterations. The inner loop, however, lacks\nthis guard: when it scans forward looking for a matching peer, it can\nencounter a slot that was NULLed by a previous outer-loop iteration and\ndereference it unconditionally.\n\nIn practice this requires at least two broken pairs whose peers\nboth survive rename/copy detection and appear later in the queue,\nwhich is rare but not impossible.\n\nAdd the same `if (!pp) continue` guard to the inner loop.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n diffcore-break.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/diffcore-break.c b/diffcore-break.c\nindex 17b5ad1fed..b5bcc956cc 100644\n--- a/diffcore-break.c\n+++ b/diffcore-break.c\n@@ -289,6 +289,8 @@ void diffcore_merge_broken(void)\n \t\t\t */\n \t\t\tfor (j = i + 1; j < q->nr; j++) {\n \t\t\t\tstruct diff_filepair *pp = q->queue[j];\n+\t\t\t\tif (!pp)\n+\t\t\t\t\tcontinue;\n \t\t\t\tif (pp->broken_pair &&\n \t\t\t\t !strcmp(pp->one->path, pp->two->path) &&\n \t\t\t\t !strcmp(p->one->path, pp->two->path)) {\n-- \ngitgitgadget\n\n"},{"id":"547586","messageId":"4fdba0542b3d643affe32ec35f27fdbabccf54d0.1783590159.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH 02/11] diff: handle NULL return from repo_get_commit_tree()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:29Z","receivedAt":"2026-07-09T09:42:43Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe `repo_get_commit_tree()` function can return NULL when a commit's\ntree object is not available (e.g., the commit was parsed but its\nmaybe_tree field is unset and the commit is not in the commit-graph). In\ncmd_diff(), the return value is immediately dereferenced via ->object\nwithout a NULL check, which would crash if the tree cannot be loaded.\n\nAdd an explicit NULL check and die with a descriptive message.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/diff.c | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 4b46e394ce..18b1083e98 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -579,9 +579,13 @@ int cmd_diff(int argc,\n \t\tobj = deref_tag(the_repository, obj, NULL, 0);\n \t\tif (!obj)\n \t\t\tdie(_(\"invalid object '%s' given.\"), name);\n-\t\tif (obj->type == OBJ_COMMIT)\n-\t\t\tobj = &repo_get_commit_tree(the_repository,\n-\t\t\t\t\t\t ((struct commit *)obj))->object;\n+\t\tif (obj->type == OBJ_COMMIT) {\n+\t\t\tstruct tree *tree = repo_get_commit_tree(\n+\t\t\t\tthe_repository, (struct commit *)obj);\n+\t\t\tif (!tree)\n+\t\t\t\tdie(_(\"unable to read tree object for commit '%s'\"), name);\n+\t\t\tobj = &tree->object;\n+\t\t}\n \n \t\tif (obj->type == OBJ_TREE) {\n \t\t\tif (sdiff.skip && bitmap_get(sdiff.skip, i))\n-- \ngitgitgadget\n\n"},{"id":"547587","messageId":"dcaefc598779123cea19807877e074acb3e1575a.1783590159.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH 03/11] remote: guard `remote_tracking()` against NULL remote","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:30Z","receivedAt":"2026-07-09T09:42:45Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe `remote_tracking()` function unconditionally dereferences\n`remote->fetch` without checking whether remote is NULL.\n\nIn practice, this never happens because the only caller (`apply_cas()`)\nguards the calls to this function by checking the `use_tracking` and\n`use_tracking_for_rest` attributes.\n\nHowever, it requires quite involved reasoning to reach that conclusion,\nand is therefore fragile. Just return -1 (\"no tracking ref\") when there\nis no remote to work with.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n remote.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/remote.c b/remote.c\nindex 00723b385e..34d0367f11 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -2681,6 +2681,8 @@ static int remote_tracking(struct remote *remote, const char *refname,\n {\n \tchar *dst;\n \n+\tif (!remote)\n+\t\treturn -1; /* no remote to look up tracking ref */\n \tdst = apply_refspecs(&remote->fetch, refname);\n \tif (!dst)\n \t\treturn -1; /* no tracking ref for refname at remote */\n-- \ngitgitgadget\n\n"},{"id":"547588","messageId":"d7bc7fce35bb169a20a4ae9a1630e7080e133b23.1783590159.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH 04/11] reftable/stack: guard against NULL list_file in stack_destroy","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:31Z","receivedAt":"2026-07-09T09:42:46Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen reftable_new_stack() fails partway through initialization\n(e.g., reftable_buf_addstr returns an OOM error before\nreftable_buf_detach assigns p->list_file), it jumps to the error\npath which calls reftable_stack_destroy(p). At that point,\np->list_file is still NULL because the detach never happened.\n\nreftable_stack_destroy() passes st->list_file unconditionally to\nread_lines(), which calls open(filename, O_RDONLY). Passing NULL\nto open() is undefined behavior and will typically crash.\n\nGuard the read_lines() call with a NULL check on st->list_file.\nWhen list_file is NULL, there are no table files to clean up\nanyway, so skipping read_lines is the correct behavior.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n reftable/stack.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/stack.c b/reftable/stack.c\nindex 1fba96ddb3..3fc3c0b2d1 100644\n--- a/reftable/stack.c\n+++ b/reftable/stack.c\n@@ -171,7 +171,8 @@ void reftable_stack_destroy(struct reftable_stack *st)\n \t\tst->merged = NULL;\n \t}\n \n-\terr = read_lines(st->list_file, &names);\n+\tif (st->list_file)\n+\t\terr = read_lines(st->list_file, &names);\n \tif (err < 0) {\n \t\tREFTABLE_FREE_AND_NULL(names);\n \t}\n-- \ngitgitgadget\n\n"},{"id":"547589","messageId":"41eef047ae6e3c332e1c8f96a9f9abf55d5004fc.1783590159.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH 05/11] mailsplit: move NULL check before first use of file handle","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:32Z","receivedAt":"2026-07-09T09:42:48Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe `split_mbox()` function calls fileno(f) to check whether the input\nis a terminal, but the NULL check for f (from `fopen()`) does not happen\nuntil later. When the file cannot be opened, f is NULL, and\n`fileno(NULL)` is undefined behavior, typically crashing with a\nsegmentation fault.\n\nMove the NULL check above the `isatty()`/`fileno()` call so the error\npath is taken before any use of the potentially-NULL handle.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/mailsplit.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/mailsplit.c b/builtin/mailsplit.c\nindex 264df6259a..0993418e63 100644\n--- a/builtin/mailsplit.c\n+++ b/builtin/mailsplit.c\n@@ -225,14 +225,14 @@ static int split_mbox(const char *file, const char *dir, int allow_bare,\n \tFILE *f = !strcmp(file, \"-\") ? stdin : fopen(file, \"r\");\n \tint file_done = 0;\n \n-\tif (isatty(fileno(f)))\n-\t\twarning(_(\"reading patches from stdin/tty...\"));\n-\n \tif (!f) {\n \t\terror_errno(\"cannot open mbox %s\", file);\n \t\tgoto out;\n \t}\n \n+\tif (isatty(fileno(f)))\n+\t\twarning(_(\"reading patches from stdin/tty...\"));\n+\n \tdo {\n \t\tpeek = fgetc(f);\n \t\tif (peek == EOF) {\n-- \ngitgitgadget\n\n"},{"id":"547590","messageId":"704137510808ade246c6f1463e88a8e3041e0f7d.1783590159.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH 06/11] bisect: handle NULL commit in `bisect_successful()`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:33Z","receivedAt":"2026-07-09T09:42:49Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen `lookup_commit_reference_by_name()` is called to find the first bad\ncommit, the result is passed to `repo_format_commit_message()`\nimmediately, which dereferences commit without checking for NULL.\n\nHowever, the commit could be NULL, even though in practice this is\nunlikely because `bisect_successful()` is only called after a successful\nbisect run has identified the bad commit, but the ref could still become\ndangling due to a concurrent gc or repository corruption.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex e7c2d2f3bb..6ff600c856 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -663,6 +663,11 @@ static int bisect_successful(struct bisect_terms *terms)\n \n \trefs_read_ref(get_main_ref_store(the_repository), bad_ref, &oid);\n \tcommit = lookup_commit_reference_by_name(bad_ref);\n+\tif (!commit) {\n+\t\tres = error(_(\"could not find commit for '%s'\"), bad_ref);\n+\t\tfree(bad_ref);\n+\t\treturn res;\n+\t}\n \trepo_format_commit_message(the_repository, commit, \"%s\", &commit_name,\n \t\t\t\t &pp);\n \n-- \ngitgitgadget\n\n"},{"id":"547591","messageId":"a7245cdffad651a423a4014c176f68c47231c62b.1783590159.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH 07/11] replay: die when --onto does not peel to a commit","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:34Z","receivedAt":"2026-07-09T09:42:50Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe `peel_committish()` function calls `repo_peel_to_type()` to convert\nthe given object to a commit, but does not check the return value. When\nthe object exists but cannot be peeled to a commit (e.g., a tree or blob\nOID is passed as --onto), the return value is NULL. Add an explicit NULL\ncheck and die with a descriptive message in that case.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n replay.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/replay.c b/replay.c\nindex da531d5bc6..b38cd5efe4 100644\n--- a/replay.c\n+++ b/replay.c\n@@ -36,12 +36,16 @@ static struct commit *peel_committish(struct repository *repo,\n {\n \tstruct object *obj;\n \tstruct object_id oid;\n+\tstruct commit *commit;\n \n \tif (repo_get_oid(repo, name, &oid))\n \t\tdie(_(\"'%s' is not a valid commit-ish for %s\"), name, mode);\n \tobj = parse_object_or_die(repo, &oid, name);\n-\treturn (struct commit *)repo_peel_to_type(repo, name, 0, obj,\n-\t\t\t\t\t\t OBJ_COMMIT);\n+\tcommit = (struct commit *)repo_peel_to_type(repo, name, 0, obj,\n+\t\t\t\t\t\t OBJ_COMMIT);\n+\tif (!commit)\n+\t\tdie(_(\"'%s' does not point to a commit for %s\"), name, mode);\n+\treturn commit;\n }\n \n static char *get_author(const char *message)\n-- \ngitgitgadget\n\n"},{"id":"547592","messageId":"0675767797f103b79ab936e01bfd06747725bcad.1783590159.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH 08/11] revision: avoid dereferencing NULL in `add_parents_only()`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:35Z","receivedAt":"2026-07-09T09:42:51Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThis function resolves revision suffixes like commit^@ (all parents),\ncommit^! (commit minus parents), and commit^-N (exclude Nth parent). It\ncalls `get_reference()` in a loop to peel through tag objects until it\nreaches a commit.\n\nThe existing NULL check after `get_reference()` only handles the\nignore_missing case, but get_reference() can return NULL through three\ndistinct paths:\n\n 1. revs->ignore_missing: the caller asked to silently skip missing\n objects.\n\n 2. revs->exclude_promisor_objects: the object is a lazy promisor\n object that should be excluded from the walk.\n\n 3. revs->do_not_die_on_missing_objects: the caller wants to record\n missing OIDs for later reporting (used by `git rev-list\n --missing=print`) rather than dying.\n\nIn the latter two instances, the code falls through to dereference the\nNULL pointer.\n\nHandle all three cases explicitly:\n\n - ignore_missing: return 0, matching the existing behavior and\n the pattern in `handle_revision_arg()`.\n\n - do_not_die_on_missing_objects: return 0. The missing OID has already\n been recorded in `revs->missing_commits` by `get_reference()`.\n Returning 0 is consistent with `handle_revision_arg()` and\n `process_parents()`, both of which continue without error when this flag\n is set. The broader codebase pattern for this flag is \"record and\n continue\": list-objects.c, builtin/rev-list.c, and process_parents\n all skip the die/error and keep walking.\n\n - everything else (only the `exclude_promisor_objects` case in\n practice): return -1, consistent with `handle_revision_arg()` where\n the condition only matches `ignore_missing` or\n `do_not_die_on_missing_objects`, falling through to ret = -1 for the\n promisor case.\n\nNote: the callers of `add_parents_only()` in\n`handle_revision_pseudo_opt()` treat any nonzero return as \"handled\"\n(`if (add_parents_only(...)) { ret = 0; }`), so the -1 for the promisor\ncase is indistinguishable from success there. This means a\npromisor-excluded tag target referenced via commit^@ would be silently\nskipped rather than producing an error. This is a pre-existing\nlimitation of the caller's return value handling and not made worse by\nthis change; the alternative (a NULL dereference crash) _would be_\nstrictly worse.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n revision.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex e91d7e1f11..7f3999b551 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1903,8 +1903,13 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,\n \t\treturn 0;\n \twhile (1) {\n \t\tit = get_reference(revs, arg, &oid, 0);\n-\t\tif (!it && revs->ignore_missing)\n-\t\t\treturn 0;\n+\t\tif (!it) {\n+\t\t\tif (revs->ignore_missing)\n+\t\t\t\treturn 0;\n+\t\t\tif (revs->do_not_die_on_missing_objects)\n+\t\t\t\treturn 0;\n+\t\t\treturn -1;\n+\t\t}\n \t\tif (it->type != OBJ_TAG)\n \t\t\tbreak;\n \t\tif (!((struct tag*)it)->tagged)\n-- \ngitgitgadget\n\n"},{"id":"547593","messageId":"0b27860478a284719755b8ac2386862c1fc3d0e7.1783590159.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH 09/11] pack-bitmap: handle missing bitmap for base MIDX","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:36Z","receivedAt":"2026-07-09T09:42:52Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen `prepare_midx_bitmap_git()` is called to load the bitmap for a\nchained MIDX's base layer, if the base MIDX does not have an associated\nbitmap file (e.g., it was not generated, or was deleted by gc), the\nreturn value is NULL. It is then stored in `bitmap_git->base` and\nimmediately dereferenced on the next line.\n\nThis can happen in practice with incremental MIDX chains: the base MIDX\nmay have been written without `--write-bitmap-index`, or the bitmap may\nhave been pruned while the incremental layer's bitmap still references\nit.\n\nCheck the return value and go to the cleanup label (which unmaps the\ncurrent bitmap and returns -1) so the caller falls back to non-bitmap\nobject enumeration, matching the handling of other bitmap loading\nfailures in the same function.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n pack-bitmap.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex e8a82945cc..ca7998c10b 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -523,6 +523,10 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,\n \n \tif (midx->base_midx) {\n \t\tbitmap_git->base = prepare_midx_bitmap_git(midx->base_midx);\n+\t\tif (!bitmap_git->base) {\n+\t\t\twarning(_(\"could not open bitmap for base MIDX\"));\n+\t\t\tgoto cleanup;\n+\t\t}\n \t\tbitmap_git->base_nr = bitmap_git->base->base_nr + 1;\n \t} else {\n \t\tbitmap_git->base_nr = 0;\n-- \ngitgitgadget\n\n"},{"id":"547594","messageId":"428a3a006bbcb165a96495bbc2c5fc04e5b15db4.1783590159.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH 10/11] bisect: ensure non-NULL `head` before using it","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:37Z","receivedAt":"2026-07-09T09:42:53Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen `refs_resolve_ref_unsafe()` is called to resolve HEAD, and returns\nNULL (e.g., HEAD does not exist as a proper ref), the code falls back to\n`repo_get_oid(\"HEAD\")` to try to resolve the OID directly. If that\nsucceeds, execution continues with `head` still set to NULL.\n\nLater, that variable is passed to `repo_get_oid()` and `starts_with()`,\nboth of which would dereference the NULL pointer.\n\nThe scenario \"`refs_resolve_ref_unsafe()` returns NULL but\n`repo_get_oid()` succeeds\" can happen when HEAD is a detached bare OID\nthat the ref backend cannot resolve symbolically (a potential edge case\nwith the reftable backend) but the OID itself is valid. In this case,\nthe bisect-start file does not yet exist (this is a fresh \"git bisect\nstart\"), so the else branch is taken with the NULL `head`.\n\nSimply assign \"HEAD\" to `head` as a fallback to address this.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 6ff600c856..a69771c6d3 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -811,9 +811,11 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,\n \t */\n \thead = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n \t\t\t\t \"HEAD\", 0, &head_oid, &flags);\n-\tif (!head)\n+\tif (!head) {\n \t\tif (repo_get_oid(the_repository, \"HEAD\", &head_oid))\n \t\t\treturn error(_(\"bad HEAD - I need a HEAD\"));\n+\t\thead = \"HEAD\";\n+\t}\n \n \t/*\n \t * Check if we are bisecting\n-- \ngitgitgadget\n\n"},{"id":"547595","messageId":"9f3a23948475eaa382e9507543fe08d933a4a461.1783590159.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH 11/11] shallow: fix NULL dereference","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-09T09:42:38Z","receivedAt":"2026-07-09T09:42:54Z","isPatch":true,"body":"From: Johannes Schindelin \n\nAfter `write_one_shallow()` calls `lookup_commit()` to find the commit\nobject for a shallow graft entry, it then checks `if (!c || ...)`.\nInside that block, when the VERBOSE flag is set, it prints the OID being\nremoved, via `c->object.oid`. But `c` can be NULL (the first condition\nin the `||` check).\n\nThis happens when a shallow graft entry references a commit object that\nis not in the object store (e.g., after a partial fetch or in a\ncorrupted repository). In that case, `lookup_commit()` returns NULL\nbecause the object cannot be found, the SEEN_ONLY check correctly\ndecides to remove this entry from .git/shallow, but the verbose message\ncrashes before the removal can complete.\n\nUse `graft->oid` instead of `c->object.oid` for the message. The graft\nentry's OID is the same value (it was used as the lookup key) and is\nalways available regardless of whether the commit object exists.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n shallow.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/shallow.c b/shallow.c\nindex 07cae44ae5..3d2230351e 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -371,7 +371,7 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)\n \t\tif (!c || !(c->object.flags & SEEN)) {\n \t\t\tif (data->flags & VERBOSE)\n \t\t\t\tprintf(\"Removing %s from .git/shallow\\n\",\n-\t\t\t\t oid_to_hex(&c->object.oid));\n+\t\t\t\t oid_to_hex(&graft->oid));\n \t\t\treturn 0;\n \t\t}\n \t}\n-- \ngitgitgadget\n"},{"id":"547647","messageId":"xmqqa4rzlyaj.fsf@gitster.g","threadId":"65960","inReplyTo":"9f3a23948475eaa382e9507543fe08d933a4a461.1783590159.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 11/11] shallow: fix NULL dereference","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-09T20:10:12Z","receivedAt":"2026-07-09T20:10:14Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> diff --git a/shallow.c b/shallow.c\n> index 07cae44ae5..3d2230351e 100644\n> --- a/shallow.c\n> +++ b/shallow.c\n> @@ -371,7 +371,7 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)\n> \t\tif (!c || !(c->object.flags & SEEN)) {\n> \t\t\tif (data->flags & VERBOSE)\n> \t\t\t\tprintf(\"Removing %s from .git/shallow\\n\",\n> -\t\t\t\t oid_to_hex(&c->object.oid));\n> +\t\t\t\t oid_to_hex(&graft->oid));\n> \t\t\treturn 0;\n\nHaha. We come into this block and emit this message when we may not\neven have a valid 'c', yet we use c->object.oid there. It makes\nperfect sense to use graft->oid here instead, as your patch does.\n\nHowever, its hexadecimal representation has already been computed in\nthe local variable 'hex', and the \"happy path\" code after this\nsection seems to assume that 'hex' is still valid (even though\noid_to_hex() uses rotating 4-element buffer, which makes the\nassumption a risky one).\n\nWe should use \"hex\" here instead of oid_to_hex(&graft->oid), which\ndoes not add to the existing risk. In addition, if we add something\nlike:\n\n struct write_shallow_data *data = cb_data;\n -\tconst char *hex = oid_to_hex(&graft->oid);\n +\tchar hex[GIT_MAX_HEXSZ + 1];\n +\n + oid_to_hex_r(hex, &graft->oid);\n if (graft->nr_parent != -1)\n return 0;\n\nto the beginning of the function, we can get rid of existing\nriskiness entirely.\n"},{"id":"547661","messageId":"xmqqpl0vh73t.fsf@gitster.g","threadId":"65960","inReplyTo":"df00334f8b8cb85a928e1ca22aa12dd6b87fb154.1783590159.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 01/11] diffcore-break: guard against NULLed queue entries in merge loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T03:11:02Z","receivedAt":"2026-07-10T03:11:05Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> The outer loop in `diffcore_merge_broken()` sets `q->queue[j]` to NULL\n> when it merges a broken pair back together, and has a NULL check to skip\n> such entries on subsequent iterations. The inner loop, however, lacks\n> this guard: when it scans forward looking for a matching peer, it can\n> encounter a slot that was NULLed by a previous outer-loop iteration and\n> dereference it unconditionally.\n>\n> In practice this requires at least two broken pairs whose peers\n> both survive rename/copy detection and appear later in the queue,\n> which is rare but not impossible.\n\nInteresting find. This is an ancient part of the codebase that\nnobody has touched in the past 21 years since eeaa460314 ([PATCH]\ndiff: Update -B heuristics., 2005-06-03) introduced it ;-).\n\nWell spotted.\n\n> Add the same `if (!pp) continue` guard to the inner loop.\n>\n> Pointed out by Coverity.\n>\n> Assisted-by: Claude Opus 4.6\n> Signed-off-by: Johannes Schindelin \n> ---\n> diffcore-break.c | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/diffcore-break.c b/diffcore-break.c\n> index 17b5ad1fed..b5bcc956cc 100644\n> --- a/diffcore-break.c\n> +++ b/diffcore-break.c\n> @@ -289,6 +289,8 @@ void diffcore_merge_broken(void)\n> \t\t\t */\n> \t\t\tfor (j = i + 1; j < q->nr; j++) {\n> \t\t\t\tstruct diff_filepair *pp = q->queue[j];\n> +\t\t\t\tif (!pp)\n> +\t\t\t\t\tcontinue;\n> \t\t\t\tif (pp->broken_pair &&\n> \t\t\t\t !strcmp(pp->one->path, pp->two->path) &&\n> \t\t\t\t !strcmp(p->one->path, pp->two->path)) {\n"},{"id":"547662","messageId":"xmqqldbjh73r.fsf@gitster.g","threadId":"65960","inReplyTo":"4fdba0542b3d643affe32ec35f27fdbabccf54d0.1783590159.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 02/11] diff: handle NULL return from repo_get_commit_tree()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T03:11:04Z","receivedAt":"2026-07-10T03:11:06Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> diff --git a/builtin/diff.c b/builtin/diff.c\n> index 4b46e394ce..18b1083e98 100644\n> --- a/builtin/diff.c\n> +++ b/builtin/diff.c\n> @@ -579,9 +579,13 @@ int cmd_diff(int argc,\n> \t\tobj = deref_tag(the_repository, obj, NULL, 0);\n> \t\tif (!obj)\n> \t\t\tdie(_(\"invalid object '%s' given.\"), name);\n> -\t\tif (obj->type == OBJ_COMMIT)\n> -\t\t\tobj = &repo_get_commit_tree(the_repository,\n> -\t\t\t\t\t\t ((struct commit *)obj))->object;\n> +\t\tif (obj->type == OBJ_COMMIT) {\n> +\t\t\tstruct tree *tree = repo_get_commit_tree(\n> +\t\t\t\tthe_repository, (struct commit *)obj);\n> +\t\t\tif (!tree)\n> +\t\t\t\tdie(_(\"unable to read tree object for commit '%s'\"), name);\n> +\t\t\tobj = &tree->object;\n> +\t\t}\n\nObviously correct.\n\n> \t\tif (obj->type == OBJ_TREE) {\n> \t\t\tif (sdiff.skip && bitmap_get(sdiff.skip, i))\n"},{"id":"547663","messageId":"xmqqh5m7h6n5.fsf@gitster.g","threadId":"65960","inReplyTo":"dcaefc598779123cea19807877e074acb3e1575a.1783590159.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 03/11] remote: guard `remote_tracking()` against NULL remote","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T03:21:02Z","receivedAt":"2026-07-10T03:21:05Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> However, it requires quite involved reasoning to reach that conclusion,\n> and is therefore fragile. Just return -1 (\"no tracking ref\") when there\n> is no remote to work with.\n\nIn a case like this, where the function is designed not to be called\nwith NULL remote, I would prefer to have an explicit BUG() rather\nthan sweeping the problem under the rug. That would make sure your\ninvestigation and involved reasoning done here remain relevant if\nthe BUG() triggers due to careless changes to the caller in the\nfuture.\n\nThanks.\n\n> Pointed out by Coverity.\n>\n> Assisted-by: Claude Opus 4.6\n> Signed-off-by: Johannes Schindelin \n> ---\n> remote.c | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/remote.c b/remote.c\n> index 00723b385e..34d0367f11 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -2681,6 +2681,8 @@ static int remote_tracking(struct remote *remote, const char *refname,\n> {\n> \tchar *dst;\n> \n> +\tif (!remote)\n> +\t\treturn -1; /* no remote to look up tracking ref */\n> \tdst = apply_refspecs(&remote->fetch, refname);\n> \tif (!dst)\n> \t\treturn -1; /* no tracking ref for refname at remote */\n"},{"id":"547664","messageId":"xmqqcxwvh6n2.fsf@gitster.g","threadId":"65960","inReplyTo":"d7bc7fce35bb169a20a4ae9a1630e7080e133b23.1783590159.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 04/11] reftable/stack: guard against NULL list_file in stack_destroy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T03:21:05Z","receivedAt":"2026-07-10T03:21:07Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> When reftable_new_stack() fails partway through initialization\n> (e.g., reftable_buf_addstr returns an OOM error before\n> reftable_buf_detach assigns p->list_file), it jumps to the error\n> path which calls reftable_stack_destroy(p). At that point,\n> p->list_file is still NULL because the detach never happened.\n>\n> reftable_stack_destroy() passes st->list_file unconditionally to\n> read_lines(), which calls open(filename, O_RDONLY). Passing NULL\n> to open() is undefined behavior and will typically crash.\n>\n> Guard the read_lines() call with a NULL check on st->list_file.\n> When list_file is NULL, there are no table files to clean up\n> anyway, so skipping read_lines is the correct behavior.\n\nNice spotting and recovery. Well done.\n\n>\n> Pointed out by Coverity.\n>\n> Assisted-by: Claude Opus 4.6\n> Signed-off-by: Johannes Schindelin \n> ---\n> reftable/stack.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/reftable/stack.c b/reftable/stack.c\n> index 1fba96ddb3..3fc3c0b2d1 100644\n> --- a/reftable/stack.c\n> +++ b/reftable/stack.c\n> @@ -171,7 +171,8 @@ void reftable_stack_destroy(struct reftable_stack *st)\n> \t\tst->merged = NULL;\n> \t}\n> \n> -\terr = read_lines(st->list_file, &names);\n> +\tif (st->list_file)\n> +\t\terr = read_lines(st->list_file, &names);\n> \tif (err < 0) {\n> \t\tREFTABLE_FREE_AND_NULL(names);\n> \t}\n"},{"id":"547665","messageId":"xmqq8q7jh66h.fsf@gitster.g","threadId":"65960","inReplyTo":"41eef047ae6e3c332e1c8f96a9f9abf55d5004fc.1783590159.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 05/11] mailsplit: move NULL check before first use of file handle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T03:31:02Z","receivedAt":"2026-07-10T03:31:05Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c\n> index 264df6259a..0993418e63 100644\n> --- a/builtin/mailsplit.c\n> +++ b/builtin/mailsplit.c\n> @@ -225,14 +225,14 @@ static int split_mbox(const char *file, const char *dir, int allow_bare,\n> \tFILE *f = !strcmp(file, \"-\") ? stdin : fopen(file, \"r\");\n> \tint file_done = 0;\n> \n> -\tif (isatty(fileno(f)))\n> -\t\twarning(_(\"reading patches from stdin/tty...\"));\n> -\n> \tif (!f) {\n> \t\terror_errno(\"cannot open mbox %s\", file);\n> \t\tgoto out;\n> \t}\n> \n> +\tif (isatty(fileno(f)))\n> +\t\twarning(_(\"reading patches from stdin/tty...\"));\n> +\n> \tdo {\n> \t\tpeek = fgetc(f);\n> \t\tif (peek == EOF) {\n\nAh, obviously correct. Cannot believe nobody noticed this since it\nwas first written in 7b20af6a06 (am/apply: warn if we end up reading\npatches from terminal, 2022-03-03).\n\nThanks.\n"},{"id":"547666","messageId":"xmqq4ii7h66f.fsf@gitster.g","threadId":"65960","inReplyTo":"704137510808ade246c6f1463e88a8e3041e0f7d.1783590159.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 06/11] bisect: handle NULL commit in `bisect_successful()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T03:31:04Z","receivedAt":"2026-07-10T03:31:06Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> diff --git a/builtin/bisect.c b/builtin/bisect.c\n> index e7c2d2f3bb..6ff600c856 100644\n> --- a/builtin/bisect.c\n> +++ b/builtin/bisect.c\n> @@ -663,6 +663,11 @@ static int bisect_successful(struct bisect_terms *terms)\n> \n> \trefs_read_ref(get_main_ref_store(the_repository), bad_ref, &oid);\n> \tcommit = lookup_commit_reference_by_name(bad_ref);\n> +\tif (!commit) {\n> +\t\tres = error(_(\"could not find commit for '%s'\"), bad_ref);\n> +\t\tfree(bad_ref);\n> +\t\treturn res;\n> +\t}\n\nCatching this case as an error is the right thing to do, but there is\na bit of an impedance mismatch between the return value from error()\nand the status passed around in the bisect codebase.\n\nThe bisect.h header defines an enum bisect_error type, and I think\nthe sole caller of this function, bisect_next(), expects to see\nBISECT_FAILED. It may happen to be the same -1 that error()\nreturns, but for longer term maintainability, I would prefer to see\nit done more like:\n\n\terror(_(\"...\"));\n\tfree(bad_ref);\n\treturn BISECT_FAILED;\n\nor something along those lines.\n\nThanks.\n\n> \trepo_format_commit_message(the_repository, commit, \"%s\", &commit_name,\n> \t\t\t\t &pp);\n"},{"id":"547667","messageId":"xmqqzezzfr5d.fsf@gitster.g","threadId":"65960","inReplyTo":"0675767797f103b79ab936e01bfd06747725bcad.1783590159.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 08/11] revision: avoid dereferencing NULL in `add_parents_only()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T03:41:02Z","receivedAt":"2026-07-10T03:41:05Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> This function resolves revision suffixes like commit^@ (all parents),\n> commit^! (commit minus parents), and commit^-N (exclude Nth parent). It\n> calls `get_reference()` in a loop to peel through tag objects until it\n> reaches a commit.\n>\n> The existing NULL check after `get_reference()` only handles the\n> ignore_missing case, but get_reference() can return NULL through three\n> distinct paths:\n\nNicely spotted. It sounds like something a test can ensure does not\nto regress in the future, unless I am misreading this explanation.\nCould you include such a test?\n\nThanks.\n\n> diff --git a/revision.c b/revision.c\n> index e91d7e1f11..7f3999b551 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -1903,8 +1903,13 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,\n> \t\treturn 0;\n> \twhile (1) {\n> \t\tit = get_reference(revs, arg, &oid, 0);\n> -\t\tif (!it && revs->ignore_missing)\n> -\t\t\treturn 0;\n> +\t\tif (!it) {\n> +\t\t\tif (revs->ignore_missing)\n> +\t\t\t\treturn 0;\n> +\t\t\tif (revs->do_not_die_on_missing_objects)\n> +\t\t\t\treturn 0;\n> +\t\t\treturn -1;\n> +\t\t}\n> \t\tif (it->type != OBJ_TAG)\n> \t\t\tbreak;\n> \t\tif (!((struct tag*)it)->tagged)\n"},{"id":"547668","messageId":"xmqqv7anfr5b.fsf@gitster.g","threadId":"65960","inReplyTo":"0b27860478a284719755b8ac2386862c1fc3d0e7.1783590159.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 09/11] pack-bitmap: handle missing bitmap for base MIDX","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T03:41:04Z","receivedAt":"2026-07-10T03:41:06Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> This can happen in practice with incremental MIDX chains: the base MIDX\n> may have been written without `--write-bitmap-index`, or the bitmap may\n> have been pruned while the incremental layer's bitmap still references\n> it.\n>\n> Check the return value and go to the cleanup label (which unmaps the\n> current bitmap and returns -1) so the caller falls back to non-bitmap\n> object enumeration, matching the handling of other bitmap loading\n> failures in the same function.\n\nNicely reasoned. It would have been nicer to CC those who are more\nfamiliar with the area, though.\n\nCc'ed Taylor for incremental MIDX expertise just in case.\n\nThanks.\n\n>\n> Pointed out by Coverity.\n>\n> Assisted-by: Claude Opus 4.6\n> Signed-off-by: Johannes Schindelin \n> ---\n> pack-bitmap.c | 4 ++++\n> 1 file changed, 4 insertions(+)\n>\n> diff --git a/pack-bitmap.c b/pack-bitmap.c\n> index e8a82945cc..ca7998c10b 100644\n> --- a/pack-bitmap.c\n> +++ b/pack-bitmap.c\n> @@ -523,6 +523,10 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,\n> \n> \tif (midx->base_midx) {\n> \t\tbitmap_git->base = prepare_midx_bitmap_git(midx->base_midx);\n> +\t\tif (!bitmap_git->base) {\n> +\t\t\twarning(_(\"could not open bitmap for base MIDX\"));\n> +\t\t\tgoto cleanup;\n> +\t\t}\n> \t\tbitmap_git->base_nr = bitmap_git->base->base_nr + 1;\n> \t} else {\n> \t\tbitmap_git->base_nr = 0;\n"},{"id":"547671","messageId":"xmqqqzlbfq81.fsf@gitster.g","threadId":"65960","inReplyTo":"428a3a006bbcb165a96495bbc2c5fc04e5b15db4.1783590159.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 10/11] bisect: ensure non-NULL `head` before using it","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T04:01:02Z","receivedAt":"2026-07-10T04:01:05Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> When `refs_resolve_ref_unsafe()` is called to resolve HEAD, and returns\n> NULL (e.g., HEAD does not exist as a proper ref), the code falls back to\n> `repo_get_oid(\"HEAD\")` to try to resolve the OID directly. If that\n> succeeds, execution continues with `head` still set to NULL.\n>\n> Later, that variable is passed to `repo_get_oid()` and `starts_with()`,\n> both of which would dereference the NULL pointer.\n>\n> The scenario \"`refs_resolve_ref_unsafe()` returns NULL but\n> `repo_get_oid()` succeeds\" can happen when HEAD is a detached bare OID\n> that the ref backend cannot resolve symbolically (a potential edge case\n> with the reftable backend) but the OID itself is valid. In this case,\n> the bisect-start file does not yet exist (this is a fresh \"git bisect\n> start\"), so the else branch is taken with the NULL `head`.\n\nI agree that setting head to the string \"HEAD\" is a good solution to\nensure that !starts_with(), !repo_get_oid(), and skip_prefix() are\nnot called with NULL.\n\nHowever, I am not sure I understand your \"can happen\" scenario.\n\nI naively thought that the only case where HEAD does not resolve to\nan object correctly is when HEAD is a symbolic ref pointing to an\nunborn branch.\n\nIs the bug in your \"can happen\" scenario something we can\ndemonstrate? If so, could you add a test to prevent regressions in\nthe future?\n\nThanks.\n\n\n> Simply assign \"HEAD\" to `head` as a fallback to address this.\n>\n> Pointed out by Coverity.\n>\n> Assisted-by: Claude Opus 4.6\n> Signed-off-by: Johannes Schindelin \n> ---\n> builtin/bisect.c | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/bisect.c b/builtin/bisect.c\n> index 6ff600c856..a69771c6d3 100644\n> --- a/builtin/bisect.c\n> +++ b/builtin/bisect.c\n> @@ -811,9 +811,11 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,\n> \t */\n> \thead = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n> \t\t\t\t \"HEAD\", 0, &head_oid, &flags);\n> -\tif (!head)\n> +\tif (!head) {\n> \t\tif (repo_get_oid(the_repository, \"HEAD\", &head_oid))\n> \t\t\treturn error(_(\"bad HEAD - I need a HEAD\"));\n> +\t\thead = \"HEAD\";\n> +\t}\n> \n> \t/*\n> \t * Check if we are bisecting\n"},{"id":"547719","messageId":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.git.1783590159.gitgitgadget@gmail.com","subject":"[PATCH v2 00/12] coverity: avoid dereferencing NULL","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:24Z","receivedAt":"2026-07-10T11:39:46Z","isPatch":true,"body":"This is a continuation of the effort I started in the patch series that\nbecame js/coverity-fixes. This next batch adds guards to avoid dereferencing\nNULL pointers and accessing NULL file descriptors.\n\nChanges since v1:\n\n * Calling remote_tracking() no longer returns -1 when remote is NULL, but\n instead BUG()s out.\n * bisect_successful() returns with BISECT_FAILED instead of the -1 that\n only worked by happenstance.\n * The commit \"revision: avoid dereferencing NULL in add_parents_only()\" now\n comes with a regression test.\n * The commit \"bisect: ensure non-NULL head before using it\" no longer\n claims that the fixed bug can be triggered with the current code base.\n * The missing shallow commit's OID is no longer computed twice.\n * A follow-up commit was folded into this patch series that lets\n write_one_shallow() avoid the rolling buffers of oid_to_hex(), as\n suggested by Junio. It technically does not fit the goal of this patch\n series (fixing issues pointed out by Coverity), but was asked for\n explicitly.\n\nJohannes Schindelin (12):\n diffcore-break: guard against NULLed queue entries in merge loop\n diff: handle NULL return from repo_get_commit_tree()\n remote: guard `remote_tracking()` against NULL remote\n reftable/stack: guard against NULL list_file in stack_destroy\n mailsplit: move NULL check before first use of file handle\n bisect: handle NULL commit in `bisect_successful()`\n replay: die when --onto does not peel to a commit\n revision: avoid dereferencing NULL in `add_parents_only()`\n pack-bitmap: handle missing bitmap for base MIDX\n bisect: ensure non-NULL `head` before using it\n shallow: fix NULL dereference\n shallow: give write_one_shallow() its own hex buffer\n\n builtin/bisect.c | 9 ++++++++-\n builtin/diff.c | 10 +++++++---\n builtin/mailsplit.c | 6 +++---\n diffcore-break.c | 2 ++\n pack-bitmap.c | 4 ++++\n reftable/stack.c | 3 ++-\n remote.c | 2 ++\n replay.c | 8 ++++++--\n revision.c | 9 +++++++--\n shallow.c | 7 ++++---\n t/t0410-partial-clone.sh | 18 ++++++++++++++++++\n 11 files changed, 63 insertions(+), 15 deletions(-)\n\n\nbase-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2174%2Fdscho%2Fcoverity-fixes-null-safety-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2174/dscho/coverity-fixes-null-safety-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/2174\n\nRange-diff vs v1:\n\n 1: df00334f8b = 1: df00334f8b diffcore-break: guard against NULLed queue entries in merge loop\n 2: 4fdba0542b = 2: 4fdba0542b diff: handle NULL return from repo_get_commit_tree()\n 3: dcaefc5987 ! 3: 1398a2f120 remote: guard `remote_tracking()` against NULL remote\n @@ remote.c: static int remote_tracking(struct remote *remote, const char *refname,\n \tchar *dst;\n \n +\tif (!remote)\n -+\t\treturn -1; /* no remote to look up tracking ref */\n ++\t\tBUG(\"remote_tracking() called with NULL remote\");\n \tdst = apply_refspecs(&remote->fetch, refname);\n \tif (!dst)\n \t\treturn -1; /* no tracking ref for refname at remote */\n 4: d7bc7fce35 = 4: 285f019fb3 reftable/stack: guard against NULL list_file in stack_destroy\n 5: 41eef047ae = 5: 12c2c8450e mailsplit: move NULL check before first use of file handle\n 6: 7041375108 ! 6: ca818ee405 bisect: handle NULL commit in `bisect_successful()`\n @@ builtin/bisect.c: static int bisect_successful(struct bisect_terms *terms)\n \trefs_read_ref(get_main_ref_store(the_repository), bad_ref, &oid);\n \tcommit = lookup_commit_reference_by_name(bad_ref);\n +\tif (!commit) {\n -+\t\tres = error(_(\"could not find commit for '%s'\"), bad_ref);\n ++\t\terror(_(\"could not find commit for '%s'\"), bad_ref);\n +\t\tfree(bad_ref);\n -+\t\treturn res;\n ++\t\treturn BISECT_FAILED;\n +\t}\n \trepo_format_commit_message(the_repository, commit, \"%s\", &commit_name,\n \t\t\t\t &pp);\n 7: a7245cdffa = 7: 8216769be9 replay: die when --onto does not peel to a commit\n 8: 0675767797 ! 8: 41285dd8e1 revision: avoid dereferencing NULL in `add_parents_only()`\n @@ revision.c: static int add_parents_only(struct rev_info *revs, const char *arg_,\n \t\tif (it->type != OBJ_TAG)\n \t\t\tbreak;\n \t\tif (!((struct tag*)it)->tagged)\n +\n + ## t/t0410-partial-clone.sh ##\n +@@ t/t0410-partial-clone.sh: test_expect_success 'rev-list dies for missing objects on cmd line' '\n + \tdone\n + '\n + \n ++test_expect_success '--exclude-promisor-objects with ^@ on missing object' '\n ++\trm -rf repo &&\n ++\ttest_create_repo repo &&\n ++\ttest_commit -C repo foo &&\n ++\ttest_commit -C repo bar &&\n ++\n ++\tCOMMIT=$(git -C repo rev-parse foo) &&\n ++\tpromise_and_delete \"$COMMIT\" &&\n ++\n ++\tgit -C repo config core.repositoryformatversion 1 &&\n ++\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n ++\n ++\t# Ensure that \"$COMMIT^@\" is handled gracefully even though the\n ++\t# actual commits are missing.\n ++\tgit -C repo rev-list --exclude-promisor-objects \"$COMMIT^@\" >out &&\n ++\ttest_must_be_empty out\n ++'\n ++\n + test_expect_success 'single promisor remote can be re-initialized gracefully' '\n + \t# ensure one promisor is in the promisors list\n + \trm -rf repo &&\n 9: 0b27860478 = 9: cccd36137f pack-bitmap: handle missing bitmap for base MIDX\n 10: 428a3a006b ! 10: 376a6581cb bisect: ensure non-NULL `head` before using it\n @@ Commit message\n Later, that variable is passed to `repo_get_oid()` and `starts_with()`,\n both of which would dereference the NULL pointer.\n \n - The scenario \"`refs_resolve_ref_unsafe()` returns NULL but\n - `repo_get_oid()` succeeds\" can happen when HEAD is a detached bare OID\n - that the ref backend cannot resolve symbolically (a potential edge case\n - with the reftable backend) but the OID itself is valid. In this case,\n - the bisect-start file does not yet exist (this is a fresh \"git bisect\n - start\"), so the else branch is taken with the NULL `head`.\n -\n - Simply assign \"HEAD\" to `head` as a fallback to address this.\n -\n - Pointed out by Coverity.\n -\n - Assisted-by: Claude Opus 4.6\n + A concrete trigger for `refs_resolve_ref_unsafe()` returning NULL while\n + `repo_get_oid()` succeeds could not be constructed against the ref\n + backends currently in the tree; the naive case (a symbolic HEAD pointing\n + at a nonexistent branch, in either the files or the reftable backend)\n + fails in both calls consistently and returns via the existing\n + `error(_(\"bad HEAD - I need a HEAD\"))` path. Coverity, however, flags\n + the leftover use of `head` after the outer `if (!head)` on a formal\n + reading: `head` is still NULL at that point, and both `starts_with(head,\n + ...)` and the second `repo_get_oid(..., head, ...)` in the else-branch\n + would dereference it if that state were ever reached.\n +\n + Removing the outer check would risk regressing to a crash if a future\n + ref backend ever manages to hit the \"returns NULL for HEAD but has a\n + valid OID for HEAD\" state. Assigning the literal string \"HEAD\" as a\n + safe fallback documents the intent and satisfies the analyzer without\n + changing behavior in any code path we can currently reach.\n +\n + Assisted-by: Claude Opus 4.7\n Signed-off-by: Johannes Schindelin \n \n ## builtin/bisect.c ##\n 11: 9f3a239484 ! 11: e581bc91ee shallow: fix NULL dereference\n @@ Commit message\n \n ## shallow.c ##\n @@ shallow.c: static int write_one_shallow(const struct commit_graft *graft, void *cb_data)\n + \t\tstruct commit *c = lookup_commit(the_repository, &graft->oid);\n \t\tif (!c || !(c->object.flags & SEEN)) {\n \t\t\tif (data->flags & VERBOSE)\n - \t\t\t\tprintf(\"Removing %s from .git/shallow\\n\",\n +-\t\t\t\tprintf(\"Removing %s from .git/shallow\\n\",\n -\t\t\t\t oid_to_hex(&c->object.oid));\n -+\t\t\t\t oid_to_hex(&graft->oid));\n ++\t\t\t\tprintf(\"Removing %s from .git/shallow\\n\", hex);\n \t\t\treturn 0;\n \t\t}\n \t}\n -: ---------- > 12: 2ef74b52fa shallow: give write_one_shallow() its own hex buffer\n\n-- \ngitgitgadget\n"},{"id":"547720","messageId":"4fdba0542b3d643affe32ec35f27fdbabccf54d0.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 02/12] diff: handle NULL return from repo_get_commit_tree()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:26Z","receivedAt":"2026-07-10T11:39:46Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe `repo_get_commit_tree()` function can return NULL when a commit's\ntree object is not available (e.g., the commit was parsed but its\nmaybe_tree field is unset and the commit is not in the commit-graph). In\ncmd_diff(), the return value is immediately dereferenced via ->object\nwithout a NULL check, which would crash if the tree cannot be loaded.\n\nAdd an explicit NULL check and die with a descriptive message.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/diff.c | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 4b46e394ce..18b1083e98 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -579,9 +579,13 @@ int cmd_diff(int argc,\n \t\tobj = deref_tag(the_repository, obj, NULL, 0);\n \t\tif (!obj)\n \t\t\tdie(_(\"invalid object '%s' given.\"), name);\n-\t\tif (obj->type == OBJ_COMMIT)\n-\t\t\tobj = &repo_get_commit_tree(the_repository,\n-\t\t\t\t\t\t ((struct commit *)obj))->object;\n+\t\tif (obj->type == OBJ_COMMIT) {\n+\t\t\tstruct tree *tree = repo_get_commit_tree(\n+\t\t\t\tthe_repository, (struct commit *)obj);\n+\t\t\tif (!tree)\n+\t\t\t\tdie(_(\"unable to read tree object for commit '%s'\"), name);\n+\t\t\tobj = &tree->object;\n+\t\t}\n \n \t\tif (obj->type == OBJ_TREE) {\n \t\t\tif (sdiff.skip && bitmap_get(sdiff.skip, i))\n-- \ngitgitgadget\n\n"},{"id":"547717","messageId":"1398a2f1200da5cbc716b1a926aa614ef6c13503.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 03/12] remote: guard `remote_tracking()` against NULL remote","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:27Z","receivedAt":"2026-07-10T11:39:48Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe `remote_tracking()` function unconditionally dereferences\n`remote->fetch` without checking whether remote is NULL.\n\nIn practice, this never happens because the only caller (`apply_cas()`)\nguards the calls to this function by checking the `use_tracking` and\n`use_tracking_for_rest` attributes.\n\nHowever, it requires quite involved reasoning to reach that conclusion,\nand is therefore fragile. Just return -1 (\"no tracking ref\") when there\nis no remote to work with.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n remote.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/remote.c b/remote.c\nindex 00723b385e..887e388f9c 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -2681,6 +2681,8 @@ static int remote_tracking(struct remote *remote, const char *refname,\n {\n \tchar *dst;\n \n+\tif (!remote)\n+\t\tBUG(\"remote_tracking() called with NULL remote\");\n \tdst = apply_refspecs(&remote->fetch, refname);\n \tif (!dst)\n \t\treturn -1; /* no tracking ref for refname at remote */\n-- \ngitgitgadget\n\n"},{"id":"547728","messageId":"df00334f8b8cb85a928e1ca22aa12dd6b87fb154.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 01/12] diffcore-break: guard against NULLed queue entries in merge loop","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:25Z","receivedAt":"2026-07-10T11:39:48Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe outer loop in `diffcore_merge_broken()` sets `q->queue[j]` to NULL\nwhen it merges a broken pair back together, and has a NULL check to skip\nsuch entries on subsequent iterations. The inner loop, however, lacks\nthis guard: when it scans forward looking for a matching peer, it can\nencounter a slot that was NULLed by a previous outer-loop iteration and\ndereference it unconditionally.\n\nIn practice this requires at least two broken pairs whose peers\nboth survive rename/copy detection and appear later in the queue,\nwhich is rare but not impossible.\n\nAdd the same `if (!pp) continue` guard to the inner loop.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n diffcore-break.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/diffcore-break.c b/diffcore-break.c\nindex 17b5ad1fed..b5bcc956cc 100644\n--- a/diffcore-break.c\n+++ b/diffcore-break.c\n@@ -289,6 +289,8 @@ void diffcore_merge_broken(void)\n \t\t\t */\n \t\t\tfor (j = i + 1; j < q->nr; j++) {\n \t\t\t\tstruct diff_filepair *pp = q->queue[j];\n+\t\t\t\tif (!pp)\n+\t\t\t\t\tcontinue;\n \t\t\t\tif (pp->broken_pair &&\n \t\t\t\t !strcmp(pp->one->path, pp->two->path) &&\n \t\t\t\t !strcmp(p->one->path, pp->two->path)) {\n-- \ngitgitgadget\n\n"},{"id":"547718","messageId":"12c2c8450e0f0ee71cc6beefe6186e0ededdf806.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 05/12] mailsplit: move NULL check before first use of file handle","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:29Z","receivedAt":"2026-07-10T11:39:50Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe `split_mbox()` function calls fileno(f) to check whether the input\nis a terminal, but the NULL check for f (from `fopen()`) does not happen\nuntil later. When the file cannot be opened, f is NULL, and\n`fileno(NULL)` is undefined behavior, typically crashing with a\nsegmentation fault.\n\nMove the NULL check above the `isatty()`/`fileno()` call so the error\npath is taken before any use of the potentially-NULL handle.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/mailsplit.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/mailsplit.c b/builtin/mailsplit.c\nindex 264df6259a..0993418e63 100644\n--- a/builtin/mailsplit.c\n+++ b/builtin/mailsplit.c\n@@ -225,14 +225,14 @@ static int split_mbox(const char *file, const char *dir, int allow_bare,\n \tFILE *f = !strcmp(file, \"-\") ? stdin : fopen(file, \"r\");\n \tint file_done = 0;\n \n-\tif (isatty(fileno(f)))\n-\t\twarning(_(\"reading patches from stdin/tty...\"));\n-\n \tif (!f) {\n \t\terror_errno(\"cannot open mbox %s\", file);\n \t\tgoto out;\n \t}\n \n+\tif (isatty(fileno(f)))\n+\t\twarning(_(\"reading patches from stdin/tty...\"));\n+\n \tdo {\n \t\tpeek = fgetc(f);\n \t\tif (peek == EOF) {\n-- \ngitgitgadget\n\n"},{"id":"547726","messageId":"285f019fb3c3f1d3eb6066de93315acf273dca69.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 04/12] reftable/stack: guard against NULL list_file in stack_destroy","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:28Z","receivedAt":"2026-07-10T11:39:50Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen reftable_new_stack() fails partway through initialization\n(e.g., reftable_buf_addstr returns an OOM error before\nreftable_buf_detach assigns p->list_file), it jumps to the error\npath which calls reftable_stack_destroy(p). At that point,\np->list_file is still NULL because the detach never happened.\n\nreftable_stack_destroy() passes st->list_file unconditionally to\nread_lines(), which calls open(filename, O_RDONLY). Passing NULL\nto open() is undefined behavior and will typically crash.\n\nGuard the read_lines() call with a NULL check on st->list_file.\nWhen list_file is NULL, there are no table files to clean up\nanyway, so skipping read_lines is the correct behavior.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n reftable/stack.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/stack.c b/reftable/stack.c\nindex 1fba96ddb3..3fc3c0b2d1 100644\n--- a/reftable/stack.c\n+++ b/reftable/stack.c\n@@ -171,7 +171,8 @@ void reftable_stack_destroy(struct reftable_stack *st)\n \t\tst->merged = NULL;\n \t}\n \n-\terr = read_lines(st->list_file, &names);\n+\tif (st->list_file)\n+\t\terr = read_lines(st->list_file, &names);\n \tif (err < 0) {\n \t\tREFTABLE_FREE_AND_NULL(names);\n \t}\n-- \ngitgitgadget\n\n"},{"id":"547724","messageId":"ca818ee405a8078e14ec4faab2422b34c6e83681.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 06/12] bisect: handle NULL commit in `bisect_successful()`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:30Z","receivedAt":"2026-07-10T11:39:53Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen `lookup_commit_reference_by_name()` is called to find the first bad\ncommit, the result is passed to `repo_format_commit_message()`\nimmediately, which dereferences commit without checking for NULL.\n\nHowever, the commit could be NULL, even though in practice this is\nunlikely because `bisect_successful()` is only called after a successful\nbisect run has identified the bad commit, but the ref could still become\ndangling due to a concurrent gc or repository corruption.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex e7c2d2f3bb..408e0f414e 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -663,6 +663,11 @@ static int bisect_successful(struct bisect_terms *terms)\n \n \trefs_read_ref(get_main_ref_store(the_repository), bad_ref, &oid);\n \tcommit = lookup_commit_reference_by_name(bad_ref);\n+\tif (!commit) {\n+\t\terror(_(\"could not find commit for '%s'\"), bad_ref);\n+\t\tfree(bad_ref);\n+\t\treturn BISECT_FAILED;\n+\t}\n \trepo_format_commit_message(the_repository, commit, \"%s\", &commit_name,\n \t\t\t\t &pp);\n \n-- \ngitgitgadget\n\n"},{"id":"547725","messageId":"41285dd8e1df9d010648459d5ff93db72bff7c1a.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 08/12] revision: avoid dereferencing NULL in `add_parents_only()`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:32Z","receivedAt":"2026-07-10T11:39:55Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThis function resolves revision suffixes like commit^@ (all parents),\ncommit^! (commit minus parents), and commit^-N (exclude Nth parent). It\ncalls `get_reference()` in a loop to peel through tag objects until it\nreaches a commit.\n\nThe existing NULL check after `get_reference()` only handles the\nignore_missing case, but get_reference() can return NULL through three\ndistinct paths:\n\n 1. revs->ignore_missing: the caller asked to silently skip missing\n objects.\n\n 2. revs->exclude_promisor_objects: the object is a lazy promisor\n object that should be excluded from the walk.\n\n 3. revs->do_not_die_on_missing_objects: the caller wants to record\n missing OIDs for later reporting (used by `git rev-list\n --missing=print`) rather than dying.\n\nIn the latter two instances, the code falls through to dereference the\nNULL pointer.\n\nHandle all three cases explicitly:\n\n - ignore_missing: return 0, matching the existing behavior and\n the pattern in `handle_revision_arg()`.\n\n - do_not_die_on_missing_objects: return 0. The missing OID has already\n been recorded in `revs->missing_commits` by `get_reference()`.\n Returning 0 is consistent with `handle_revision_arg()` and\n `process_parents()`, both of which continue without error when this flag\n is set. The broader codebase pattern for this flag is \"record and\n continue\": list-objects.c, builtin/rev-list.c, and process_parents\n all skip the die/error and keep walking.\n\n - everything else (only the `exclude_promisor_objects` case in\n practice): return -1, consistent with `handle_revision_arg()` where\n the condition only matches `ignore_missing` or\n `do_not_die_on_missing_objects`, falling through to ret = -1 for the\n promisor case.\n\nNote: the callers of `add_parents_only()` in\n`handle_revision_pseudo_opt()` treat any nonzero return as \"handled\"\n(`if (add_parents_only(...)) { ret = 0; }`), so the -1 for the promisor\ncase is indistinguishable from success there. This means a\npromisor-excluded tag target referenced via commit^@ would be silently\nskipped rather than producing an error. This is a pre-existing\nlimitation of the caller's return value handling and not made worse by\nthis change; the alternative (a NULL dereference crash) _would be_\nstrictly worse.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n revision.c | 9 +++++++--\n t/t0410-partial-clone.sh | 18 ++++++++++++++++++\n 2 files changed, 25 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex e91d7e1f11..7f3999b551 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1903,8 +1903,13 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,\n \t\treturn 0;\n \twhile (1) {\n \t\tit = get_reference(revs, arg, &oid, 0);\n-\t\tif (!it && revs->ignore_missing)\n-\t\t\treturn 0;\n+\t\tif (!it) {\n+\t\t\tif (revs->ignore_missing)\n+\t\t\t\treturn 0;\n+\t\t\tif (revs->do_not_die_on_missing_objects)\n+\t\t\t\treturn 0;\n+\t\t\treturn -1;\n+\t\t}\n \t\tif (it->type != OBJ_TAG)\n \t\t\tbreak;\n \t\tif (!((struct tag*)it)->tagged)\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex dff442da20..cc070019be 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -489,6 +489,24 @@ test_expect_success 'rev-list dies for missing objects on cmd line' '\n \tdone\n '\n \n+test_expect_success '--exclude-promisor-objects with ^@ on missing object' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo foo &&\n+\ttest_commit -C repo bar &&\n+\n+\tCOMMIT=$(git -C repo rev-parse foo) &&\n+\tpromise_and_delete \"$COMMIT\" &&\n+\n+\tgit -C repo config core.repositoryformatversion 1 &&\n+\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\n+\t# Ensure that \"$COMMIT^@\" is handled gracefully even though the\n+\t# actual commits are missing.\n+\tgit -C repo rev-list --exclude-promisor-objects \"$COMMIT^@\" >out &&\n+\ttest_must_be_empty out\n+'\n+\n test_expect_success 'single promisor remote can be re-initialized gracefully' '\n \t# ensure one promisor is in the promisors list\n \trm -rf repo &&\n-- \ngitgitgadget\n\n"},{"id":"547727","messageId":"8216769be9eec7489f1039e0211f3e6d3388247b.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 07/12] replay: die when --onto does not peel to a commit","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:31Z","receivedAt":"2026-07-10T11:39:55Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe `peel_committish()` function calls `repo_peel_to_type()` to convert\nthe given object to a commit, but does not check the return value. When\nthe object exists but cannot be peeled to a commit (e.g., a tree or blob\nOID is passed as --onto), the return value is NULL. Add an explicit NULL\ncheck and die with a descriptive message in that case.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n replay.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/replay.c b/replay.c\nindex da531d5bc6..b38cd5efe4 100644\n--- a/replay.c\n+++ b/replay.c\n@@ -36,12 +36,16 @@ static struct commit *peel_committish(struct repository *repo,\n {\n \tstruct object *obj;\n \tstruct object_id oid;\n+\tstruct commit *commit;\n \n \tif (repo_get_oid(repo, name, &oid))\n \t\tdie(_(\"'%s' is not a valid commit-ish for %s\"), name, mode);\n \tobj = parse_object_or_die(repo, &oid, name);\n-\treturn (struct commit *)repo_peel_to_type(repo, name, 0, obj,\n-\t\t\t\t\t\t OBJ_COMMIT);\n+\tcommit = (struct commit *)repo_peel_to_type(repo, name, 0, obj,\n+\t\t\t\t\t\t OBJ_COMMIT);\n+\tif (!commit)\n+\t\tdie(_(\"'%s' does not point to a commit for %s\"), name, mode);\n+\treturn commit;\n }\n \n static char *get_author(const char *message)\n-- \ngitgitgadget\n\n"},{"id":"547721","messageId":"cccd36137f49e7523f8ee15405574943536cb505.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 09/12] pack-bitmap: handle missing bitmap for base MIDX","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:33Z","receivedAt":"2026-07-10T11:39:56Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen `prepare_midx_bitmap_git()` is called to load the bitmap for a\nchained MIDX's base layer, if the base MIDX does not have an associated\nbitmap file (e.g., it was not generated, or was deleted by gc), the\nreturn value is NULL. It is then stored in `bitmap_git->base` and\nimmediately dereferenced on the next line.\n\nThis can happen in practice with incremental MIDX chains: the base MIDX\nmay have been written without `--write-bitmap-index`, or the bitmap may\nhave been pruned while the incremental layer's bitmap still references\nit.\n\nCheck the return value and go to the cleanup label (which unmaps the\ncurrent bitmap and returns -1) so the caller falls back to non-bitmap\nobject enumeration, matching the handling of other bitmap loading\nfailures in the same function.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n pack-bitmap.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex e8a82945cc..ca7998c10b 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -523,6 +523,10 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,\n \n \tif (midx->base_midx) {\n \t\tbitmap_git->base = prepare_midx_bitmap_git(midx->base_midx);\n+\t\tif (!bitmap_git->base) {\n+\t\t\twarning(_(\"could not open bitmap for base MIDX\"));\n+\t\t\tgoto cleanup;\n+\t\t}\n \t\tbitmap_git->base_nr = bitmap_git->base->base_nr + 1;\n \t} else {\n \t\tbitmap_git->base_nr = 0;\n-- \ngitgitgadget\n\n"},{"id":"547722","messageId":"376a6581cbdc3c5df624658f19cf19aac7694bc7.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 10/12] bisect: ensure non-NULL `head` before using it","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:34Z","receivedAt":"2026-07-10T11:39:56Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen `refs_resolve_ref_unsafe()` is called to resolve HEAD, and returns\nNULL (e.g., HEAD does not exist as a proper ref), the code falls back to\n`repo_get_oid(\"HEAD\")` to try to resolve the OID directly. If that\nsucceeds, execution continues with `head` still set to NULL.\n\nLater, that variable is passed to `repo_get_oid()` and `starts_with()`,\nboth of which would dereference the NULL pointer.\n\nA concrete trigger for `refs_resolve_ref_unsafe()` returning NULL while\n`repo_get_oid()` succeeds could not be constructed against the ref\nbackends currently in the tree; the naive case (a symbolic HEAD pointing\nat a nonexistent branch, in either the files or the reftable backend)\nfails in both calls consistently and returns via the existing\n`error(_(\"bad HEAD - I need a HEAD\"))` path. Coverity, however, flags\nthe leftover use of `head` after the outer `if (!head)` on a formal\nreading: `head` is still NULL at that point, and both `starts_with(head,\n...)` and the second `repo_get_oid(..., head, ...)` in the else-branch\nwould dereference it if that state were ever reached.\n\nRemoving the outer check would risk regressing to a crash if a future\nref backend ever manages to hit the \"returns NULL for HEAD but has a\nvalid OID for HEAD\" state. Assigning the literal string \"HEAD\" as a\nsafe fallback documents the intent and satisfies the analyzer without\nchanging behavior in any code path we can currently reach.\n\nAssisted-by: Claude Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 408e0f414e..dccf0be6bb 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -811,9 +811,11 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,\n \t */\n \thead = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n \t\t\t\t \"HEAD\", 0, &head_oid, &flags);\n-\tif (!head)\n+\tif (!head) {\n \t\tif (repo_get_oid(the_repository, \"HEAD\", &head_oid))\n \t\t\treturn error(_(\"bad HEAD - I need a HEAD\"));\n+\t\thead = \"HEAD\";\n+\t}\n \n \t/*\n \t * Check if we are bisecting\n-- \ngitgitgadget\n\n"},{"id":"547723","messageId":"e581bc91ee41731a79c1843f648a11e969c2e16a.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 11/12] shallow: fix NULL dereference","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:35Z","receivedAt":"2026-07-10T11:40:00Z","isPatch":true,"body":"From: Johannes Schindelin \n\nAfter `write_one_shallow()` calls `lookup_commit()` to find the commit\nobject for a shallow graft entry, it then checks `if (!c || ...)`.\nInside that block, when the VERBOSE flag is set, it prints the OID being\nremoved, via `c->object.oid`. But `c` can be NULL (the first condition\nin the `||` check).\n\nThis happens when a shallow graft entry references a commit object that\nis not in the object store (e.g., after a partial fetch or in a\ncorrupted repository). In that case, `lookup_commit()` returns NULL\nbecause the object cannot be found, the SEEN_ONLY check correctly\ndecides to remove this entry from .git/shallow, but the verbose message\ncrashes before the removal can complete.\n\nUse `graft->oid` instead of `c->object.oid` for the message. The graft\nentry's OID is the same value (it was used as the lookup key) and is\nalways available regardless of whether the commit object exists.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n shallow.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/shallow.c b/shallow.c\nindex 07cae44ae5..2f96db5170 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -370,8 +370,7 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)\n \t\tstruct commit *c = lookup_commit(the_repository, &graft->oid);\n \t\tif (!c || !(c->object.flags & SEEN)) {\n \t\t\tif (data->flags & VERBOSE)\n-\t\t\t\tprintf(\"Removing %s from .git/shallow\\n\",\n-\t\t\t\t oid_to_hex(&c->object.oid));\n+\t\t\t\tprintf(\"Removing %s from .git/shallow\\n\", hex);\n \t\t\treturn 0;\n \t\t}\n \t}\n-- \ngitgitgadget\n\n"},{"id":"547729","messageId":"2ef74b52fadc2bbffe7414b27db739546ab369c1.1783683577.git.gitgitgadget@gmail.com","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"[PATCH v2 12/12] shallow: give write_one_shallow() its own hex buffer","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-10T11:39:36Z","receivedAt":"2026-07-10T11:40:03Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe previous fix reuses the local `hex` variable that is already\ncomputed at the top of `write_one_shallow()`. That works today, but\n`oid_to_hex()` returns a pointer into a small rotating buffer, so it is\nnot stable across an unrelated call to `oid_to_hex()` from the same\nthread. A future edit that adds such a call between the assignment and\nthe last user of `hex` would silently corrupt the output.\n\nMove `write_one_shallow()` off the rotating buffer entirely by using a\nlocal buffer instead. The current users of that `hex` variable are\nunchanged.\n\nSuggested-by: Junio C Hamano \nAssisted-by: Claude Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n shallow.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/shallow.c b/shallow.c\nindex 2f96db5170..c567cc3c69 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -359,7 +359,9 @@ struct write_shallow_data {\n static int write_one_shallow(const struct commit_graft *graft, void *cb_data)\n {\n \tstruct write_shallow_data *data = cb_data;\n-\tconst char *hex = oid_to_hex(&graft->oid);\n+\tchar hex[GIT_MAX_HEXSZ + 1];\n+\n+\toid_to_hex_r(hex, &graft->oid);\n \tif (graft->nr_parent != -1)\n \t\treturn 0;\n \tif (data->flags & QUICK) {\n-- \ngitgitgadget\n"},{"id":"547747","messageId":"xmqqa4ryg84e.fsf@gitster.g","threadId":"65960","inReplyTo":"pull.2174.v2.git.1783683577.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 00/12] coverity: avoid dereferencing NULL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T15:46:41Z","receivedAt":"2026-07-10T15:46:43Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> This is a continuation of the effort I started in the patch series that\n> became js/coverity-fixes. This next batch adds guards to avoid dereferencing\n> NULL pointers and accessing NULL file descriptors.\n>\n> Changes since v1:\n>\n> * Calling remote_tracking() no longer returns -1 when remote is NULL, but\n> instead BUG()s out.\n> * bisect_successful() returns with BISECT_FAILED instead of the -1 that\n> only worked by happenstance.\n> * The commit \"revision: avoid dereferencing NULL in add_parents_only()\" now\n> comes with a regression test.\n> * The commit \"bisect: ensure non-NULL head before using it\" no longer\n> claims that the fixed bug can be triggered with the current code base.\n> * The missing shallow commit's OID is no longer computed twice.\n> * A follow-up commit was folded into this patch series that lets\n> write_one_shallow() avoid the rolling buffers of oid_to_hex(), as\n> suggested by Junio. It technically does not fit the goal of this patch\n> series (fixing issues pointed out by Coverity), but was asked for\n> explicitly.\n> ...\n> Range-diff vs v1:\n> ...\n\nI found everything including the new patch good. Unless others find\nmore issues in this round in a few days, let's mark the topic for\n'next'.\n\nThanks.\n"}]}