{"thread":{"id":"63464","subject":"[PATCH 01/14] revision: defensive programming","startedAt":"2025-05-15T12:45:43Z","lastAt":"2025-05-15T20:32:25Z","messageCount":19,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":14},"messages":[{"id":"518106","messageId":"pull.1890.git.1747313139.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":null,"subject":"[PATCH 00/14] Some defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:25Z","receivedAt":"2025-05-15T12:45:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"These patches implement some defensive programming to address complaints\nsome static analyzers might have.\n\nJohannes Schindelin (14):\n revision: defensive programming\n get_parent(): defensive programming\n fetch-pack: defensive programming\n unparse_commit(): defensive programming\n verify_commit_graph(): defensive programming\n stash: defensive programming\n stash: defensive programming\n push: defensive programming\n fetch: defensive programming\n describe: defensive programming\n inherit_tracking(): defensive programming\n submodule: check return value of `submodule_from_path()`\n test-tool repository: check return value of `lookup_commit()`\n shallow: handle missing shallow commits gracefully\n\n branch.c | 2 ++\n builtin/describe.c | 2 ++\n builtin/fetch.c | 3 ++-\n builtin/push.c | 2 +-\n builtin/stash.c | 7 ++++++-\n builtin/submodule--helper.c | 3 +++\n commit-graph.c | 5 +++++\n commit.c | 2 +-\n fetch-pack.c | 2 +-\n object-name.c | 2 +-\n revision.c | 3 +++\n shallow.c | 3 ++-\n t/helper/test-repository.c | 2 ++\n 13 files changed, 31 insertions(+), 7 deletions(-)\n\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1890%2Fdscho%2Fdefensive-programming-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1890/dscho/defensive-programming-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1890\n-- \ngitgitgadget\n"},{"id":"518104","messageId":"604e67ee64167efd5282b9efabbcb6e72eeaf5e6.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 01/14] revision: defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:26Z","receivedAt":"2025-05-15T12:45:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nOn the off chance that `lookup_decoration()` cannot find anything, let\n`leave_one_treesame_to_parent()` return gracefully instead of crashing.\n\nPointed out by CodeQL.\n\nSigned-off-by: Johannes Schindelin \n---\n revision.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex c4390f0938cb..59eae4eb8ba8 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -3359,6 +3359,9 @@ static int leave_one_treesame_to_parent(struct rev_info *revs, struct commit *co\n \tstruct commit_list *p;\n \tunsigned n;\n \n+\tif (!ts)\n+\t\treturn 0;\n+\n \tfor (p = commit->parents, n = 0; p; p = p->next, n++) {\n \t\tif (ts->treesame[n]) {\n \t\t\tif (p->item->object.flags & TMP_MARK) {\n-- \ngitgitgadget\n\n"},{"id":"518105","messageId":"35c4870e2c101c9ef72d1657c8f6dd077cecc5fa.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 02/14] get_parent(): defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:27Z","receivedAt":"2025-05-15T12:45:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCodeQL points out that `lookup_commit_reference()` can return NULL\nvalues.\n\nSigned-off-by: Johannes Schindelin \n---\n object-name.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/object-name.c b/object-name.c\nindex 76749fbfe652..ca54dad2f4c8 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -1106,7 +1106,7 @@ static enum get_oid_result get_parent(struct repository *r,\n \tif (ret)\n \t\treturn ret;\n \tcommit = lookup_commit_reference(r, &oid);\n-\tif (repo_parse_commit(r, commit))\n+\tif (!commit || repo_parse_commit(r, commit))\n \t\treturn MISSING_OBJECT;\n \tif (!idx) {\n \t\toidcpy(result, &commit->object.oid);\n-- \ngitgitgadget\n\n"},{"id":"518107","messageId":"9cafbd36817aabbe18197938c409edaddf7dcab2.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 03/14] fetch-pack: defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:28Z","receivedAt":"2025-05-15T12:45:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCodeQL points out that `parse_object()` can return NULL values.\n\nSigned-off-by: Johannes Schindelin \n---\n fetch-pack.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 1ed5e11dd568..4cbcb0c14c48 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -155,7 +155,7 @@ static struct commit *deref_without_lazy_fetch(const struct object_id *oid,\n \t\t\tstruct tag *tag = (struct tag *)\n \t\t\t\tparse_object(the_repository, oid);\n \n-\t\t\tif (!tag->tagged)\n+\t\t\tif (!tag || !tag->tagged)\n \t\t\t\treturn NULL;\n \t\t\tif (mark_tags_complete_and_check_obj_db)\n \t\t\t\ttag->object.flags |= COMPLETE;\n-- \ngitgitgadget\n\n"},{"id":"518108","messageId":"dcc04afb6cb0f0383c95d7fc35d492f0e1d256a9.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 04/14] unparse_commit(): defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:29Z","receivedAt":"2025-05-15T12:45:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCodeQL points out that `lookup_commit()` can return NULL values.\n\nSigned-off-by: Johannes Schindelin \n---\n commit.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/commit.c b/commit.c\nindex 6efdb03997d9..1cbc798e32ba 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -187,7 +187,7 @@ void unparse_commit(struct repository *r, const struct object_id *oid)\n {\n \tstruct commit *c = lookup_commit(r, oid);\n \n-\tif (!c->object.parsed)\n+\tif (!c || !c->object.parsed)\n \t\treturn;\n \tfree_commit_list(c->parents);\n \tc->parents = NULL;\n-- \ngitgitgadget\n\n"},{"id":"518109","messageId":"721402d9d8a76d3564aee0403ee046a2bd035dc9.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 05/14] verify_commit_graph(): defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:30Z","receivedAt":"2025-05-15T12:45:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCodeQL points out that `lookup_commit()` can return NULL values.\n\nSigned-off-by: Johannes Schindelin \n---\n commit-graph.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 1021ccb983d4..b3696736d248 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -2786,6 +2786,11 @@ static int verify_one_commit_graph(struct repository *r,\n \t\t\tthe_repository->hash_algo);\n \n \t\tgraph_commit = lookup_commit(r, &cur_oid);\n+\t\tif (!graph_commit) {\n+\t\t\tgraph_report(_(\"failed to look up commit %s for commit-graph\"),\n+\t\t\t\t oid_to_hex(&cur_oid));\n+\t\t\tcontinue;\n+\t\t}\n \t\todb_commit = (struct commit *)create_object(r, &cur_oid, alloc_commit_node(r));\n \t\tif (repo_parse_commit_internal(r, odb_commit, 0, 0)) {\n \t\t\tgraph_report(_(\"failed to parse commit %s from object database for commit-graph\"),\n-- \ngitgitgadget\n\n"},{"id":"518110","messageId":"130251bc324cc90ab19f82552888071f974c35b6.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 06/14] stash: defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:31Z","receivedAt":"2025-05-15T12:45:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCodeQL points out that `parse_tree_indirect()` can return NULL values.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/stash.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex dbaa999cf171..23c4bbd3e21e 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -285,7 +285,7 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)\n \tmemset(&opts, 0, sizeof(opts));\n \n \ttree = parse_tree_indirect(i_tree);\n-\tif (parse_tree(tree))\n+\tif (!tree || parse_tree(tree))\n \t\treturn -1;\n \n \tinit_tree_desc(t, &tree->object.oid, tree->buffer, tree->size);\n-- \ngitgitgadget\n\n"},{"id":"518111","messageId":"81e873ea2ce5180006be9ad0121d2d5aafb44730.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 07/14] stash: defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:32Z","receivedAt":"2025-05-15T12:45:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCodeQL points out that `lookup_commit()` can return NULL values.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/stash.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 23c4bbd3e21e..8efcd31d6c61 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1396,6 +1396,11 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \t\tgoto done;\n \t} else {\n \t\thead_commit = lookup_commit(the_repository, &info->b_commit);\n+\t\tif (!head_commit) {\n+\t\t\tret = error(_(\"could not look up commit '%s'\"),\n+\t\t\t\t oid_to_hex (&info->b_commit));\n+\t\t\tgoto done;\n+\t\t}\n \t}\n \n \tif (!check_changes(ps, include_untracked, &untracked_files)) {\n-- \ngitgitgadget\n\n"},{"id":"518112","messageId":"8d1efe06d9ee7a00e8408e37deea88c469f835ea.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 08/14] push: defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:33Z","receivedAt":"2025-05-15T12:45:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCodeQL points out that `branch_get()` can return NULL values.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/push.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 92d530e5c4df..db698c103424 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -90,7 +90,7 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref,\n \tif (push_default == PUSH_DEFAULT_UPSTREAM &&\n \t skip_prefix(matched->name, \"refs/heads/\", &branch_name)) {\n \t\tstruct branch *branch = branch_get(branch_name);\n-\t\tif (branch->merge_nr == 1 && branch->merge[0]->src) {\n+\t\tif (branch && branch->merge_nr == 1 && branch->merge[0]->src) {\n \t\t\trefspec_appendf(refspec, \"%s:%s\",\n \t\t\t\t\tref, branch->merge[0]->src);\n \t\t\treturn;\n-- \ngitgitgadget\n\n"},{"id":"518113","messageId":"d8397662bcf72445dd06a09a6615b066c9166cf4.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 09/14] fetch: defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:34Z","receivedAt":"2025-05-15T12:45:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCodeQL points out that `branch_get()` can return NULL values.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/fetch.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 95fd0018b981..e7523c531407 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -553,7 +553,7 @@ static struct ref *get_ref_map(struct remote *remote,\n \t\tif (remote &&\n \t\t (remote->fetch.nr ||\n \t\t /* Note: has_merge implies non-NULL branch->remote_name */\n-\t\t (has_merge && !strcmp(branch->remote_name, remote->name)))) {\n+\t\t (has_merge && branch && !strcmp(branch->remote_name, remote->name)))) {\n \t\t\tfor (i = 0; i < remote->fetch.nr; i++) {\n \t\t\t\tget_fetch_map(remote_refs, &remote->fetch.items[i], &tail, 0);\n \t\t\t\tif (remote->fetch.items[i].dst &&\n@@ -571,6 +571,7 @@ static struct ref *get_ref_map(struct remote *remote,\n \t\t\t * Note: has_merge implies non-NULL branch->remote_name\n \t\t\t */\n \t\t\tif (has_merge &&\n+\t\t\t branch &&\n \t\t\t !strcmp(branch->remote_name, remote->name))\n \t\t\t\tadd_merge_config(&ref_map, remote_refs, branch, &tail);\n \t\t} else if (!prefetch) {\n-- \ngitgitgadget\n\n"},{"id":"518114","messageId":"10393c2f951e30e6b8529ff4fafb4394c3840485.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 10/14] describe: defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:35Z","receivedAt":"2025-05-15T12:45:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCodeQL points out that `lookup_commit_reference()` can return NULL\nvalues.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/describe.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex e2e73f3d757c..455ca193ebd3 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -324,6 +324,8 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)\n \tunsigned int unannotated_cnt = 0;\n \n \tcmit = lookup_commit_reference(the_repository, oid);\n+\tif (!cmit)\n+\t\tdie(_(\"could not look up commit '%s'\"), oid_to_hex(oid));\n \n \tn = find_commit_name(&cmit->object.oid);\n \tif (n && (tags || all || n->prio == 2)) {\n-- \ngitgitgadget\n\n"},{"id":"518115","messageId":"dec21f89ffd06d8b307e7f46191be3824bd99b96.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 11/14] inherit_tracking(): defensive programming","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:36Z","receivedAt":"2025-05-15T12:45:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCodeQL points out that `branch_get()` can return NULL values.\n\nNote that the error path in this instance calls `BUG()`, not `die()`,\nfor two reasons:\n\n1. The code lives in `libgit.a` and calling `die()` from within those\n library functions is a bad practice that needs to be reduced, rather\n than increased.\n\n2. The `inherit_tracking()` function really should only be called with\n the name of an existing branch, therefore a `NULL` return value would\n indeed constitute a bug in Git's code.\n\nSigned-off-by: Johannes Schindelin \n---\n branch.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/branch.c b/branch.c\nindex 91297d55ac9f..a10b6119b214 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -224,6 +224,8 @@ static int inherit_tracking(struct tracking *tracking, const char *orig_ref)\n \tskip_prefix(orig_ref, \"refs/heads/\", &bare_ref);\n \n \tbranch = branch_get(bare_ref);\n+\tif (!branch)\n+\t\tBUG(\"could not get branch for '%s\", bare_ref);\n \tif (!branch->remote_name) {\n \t\twarning(_(\"asked to inherit tracking from '%s', but no remote is set\"),\n \t\t\tbare_ref);\n-- \ngitgitgadget\n\n"},{"id":"518116","messageId":"223a0053eb7f94dfd28db8f528b73a489f907444.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 12/14] submodule: check return value of `submodule_from_path()`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:37Z","receivedAt":"2025-05-15T12:45:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAs pointed out by CodeQL, it could be NULL and we usually check for\nthat.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/submodule--helper.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex c1a8029714bf..55826b82407c 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1934,6 +1934,9 @@ static int determine_submodule_update_strategy(struct repository *r,\n \tconst char *val;\n \tint ret;\n \n+\tif (!sub)\n+\t\treturn error(_(\"could not retrieve submodule information for path '%s'\"), path);\n+\n \tkey = xstrfmt(\"submodule.%s.update\", sub->name);\n \n \tif (update) {\n-- \ngitgitgadget\n\n"},{"id":"518117","messageId":"137f6c45af74c718e7acf0e3a569633291dcbc43.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 13/14] test-tool repository: check return value of `lookup_commit()`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:38Z","receivedAt":"2025-05-15T12:45:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nOn the off-chance that it's NULL...\n\nSigned-off-by: Johannes Schindelin \n---\n t/helper/test-repository.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/helper/test-repository.c b/t/helper/test-repository.c\nindex 63c37de33d22..05417d8f43dc 100644\n--- a/t/helper/test-repository.c\n+++ b/t/helper/test-repository.c\n@@ -27,6 +27,8 @@ static void test_parse_commit_in_graph(const char *gitdir, const char *worktree,\n \trepo_set_hash_algo(the_repository, hash_algo_by_ptr(r.hash_algo));\n \n \tc = lookup_commit(&r, commit_oid);\n+\tif (!c)\n+\t\tdie(\"Could not look up %s\", oid_to_hex(commit_oid));\n \n \tif (!parse_commit_in_graph(&r, c))\n \t\tdie(\"Couldn't parse commit\");\n-- \ngitgitgadget\n\n"},{"id":"518118","messageId":"17e9e9ae0e426728cb22927a26bbaf7191bc37cb.1747313140.git.gitgitgadget@gmail.com","threadId":"63464","inReplyTo":"pull.1890.git.1747313139.gitgitgadget@gmail.com","subject":"[PATCH 14/14] shallow: handle missing shallow commits gracefully","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-15T12:45:39Z","receivedAt":"2025-05-15T12:45:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAs pointed out by CodeQL, `lookup_commit()` can return NULL.\n\nSigned-off-by: Johannes Schindelin \n---\n shallow.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/shallow.c b/shallow.c\nindex 4bd9342c9a74..011f262cc7d4 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -702,7 +702,8 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,\n \tfor (i = 0; i < nr_shallow; i++) {\n \t\tstruct commit *c = lookup_commit(the_repository,\n \t\t\t\t\t\t &oid[shallow[i]]);\n-\t\tc->object.flags |= BOTTOM;\n+\t\tif (c)\n+\t\t\tc->object.flags |= BOTTOM;\n \t}\n \n \tfor (i = 0; i < ref->nr; i++)\n-- \ngitgitgadget\n"},{"id":"518153","messageId":"xmqq8qmx23wv.fsf@gitster.g","threadId":"63464","inReplyTo":"604e67ee64167efd5282b9efabbcb6e72eeaf5e6.1747313140.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 01/14] revision: defensive programming","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-15T17:13:20Z","receivedAt":"2025-05-15T17:13:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> On the off chance that `lookup_decoration()` cannot find anything, let\n> `leave_one_treesame_to_parent()` return gracefully instead of crashing.\n\nBut wouldn't it be a BUG(\"\") worthy event for the treesame\ndecoration not to exist for the commit object in question at this\npoint of the code? Is it really defensive to silently pretend that\nnothing bad happened and to move forward?\n\n> Pointed out by CodeQL.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> revision.c | 3 +++\n> 1 file changed, 3 insertions(+)\n>\n> diff --git a/revision.c b/revision.c\n> index c4390f0938cb..59eae4eb8ba8 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -3359,6 +3359,9 @@ static int leave_one_treesame_to_parent(struct rev_info *revs, struct commit *co\n> \tstruct commit_list *p;\n> \tunsigned n;\n> \n> +\tif (!ts)\n> +\t\treturn 0;\n> +\n> \tfor (p = commit->parents, n = 0; p; p = p->next, n++) {\n> \t\tif (ts->treesame[n]) {\n> \t\t\tif (p->item->object.flags & TMP_MARK) {\n"},{"id":"518154","messageId":"xmqq4ixl23jr.fsf@gitster.g","threadId":"63464","inReplyTo":"35c4870e2c101c9ef72d1657c8f6dd077cecc5fa.1747313140.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 02/14] get_parent(): defensive programming","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-15T17:21:12Z","receivedAt":"2025-05-15T17:21:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> CodeQL points out that `lookup_commit_reference()` can return NULL\n> values.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> object-name.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/object-name.c b/object-name.c\n> index 76749fbfe652..ca54dad2f4c8 100644\n> --- a/object-name.c\n> +++ b/object-name.c\n> @@ -1106,7 +1106,7 @@ static enum get_oid_result get_parent(struct repository *r,\n> \tif (ret)\n> \t\treturn ret;\n> \tcommit = lookup_commit_reference(r, &oid);\n> -\tif (repo_parse_commit(r, commit))\n> +\tif (!commit || repo_parse_commit(r, commit))\n> \t\treturn MISSING_OBJECT;\n\nMost of the time, the check for \"ret\" we see in the pre-context,\nwhich is a result of get_oid_1(), would prevent an oid that is not a\nvalid name for a committish to even reach this code, I would think,\nbut with possible repository corruption, we may fail to \"lookup\" the\ncommit, so this is a good correction, I would think.\n\nThanks.\n\n\n> \tif (!idx) {\n> \t\toidcpy(result, &commit->object.oid);\n"},{"id":"518155","messageId":"xmqqzffdzt0s.fsf@gitster.g","threadId":"63464","inReplyTo":"9cafbd36817aabbe18197938c409edaddf7dcab2.1747313140.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 03/14] fetch-pack: defensive programming","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-15T17:24:35Z","receivedAt":"2025-05-15T17:24:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> CodeQL points out that `parse_object()` can return NULL values.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> fetch-pack.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 1ed5e11dd568..4cbcb0c14c48 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -155,7 +155,7 @@ static struct commit *deref_without_lazy_fetch(const struct object_id *oid,\n> \t\t\tstruct tag *tag = (struct tag *)\n> \t\t\t\tparse_object(the_repository, oid);\n> \n> -\t\t\tif (!tag->tagged)\n> +\t\t\tif (!tag || !tag->tagged)\n> \t\t\t\treturn NULL;\n\nThe \"oid\" can come from corruptible sources like commit graph file,\nso I agree with your analysis that it may name a missing object in a\ncorrupt repository, leading \"tag\" being NULL. Looks correct.\n\n\n> \t\t\tif (mark_tags_complete_and_check_obj_db)\n> \t\t\t\ttag->object.flags |= COMPLETE;\n"},{"id":"518185","messageId":"20250515203223.GN3320240@coredump.intra.peff.net","threadId":"63464","inReplyTo":"35c4870e2c101c9ef72d1657c8f6dd077cecc5fa.1747313140.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 02/14] get_parent(): defensive programming","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-15T20:32:23Z","receivedAt":"2025-05-15T20:32:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 15, 2025 at 12:45:27PM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> CodeQL points out that `lookup_commit_reference()` can return NULL\n> values.\n> [...]\n> \tcommit = lookup_commit_reference(r, &oid);\n> -\tif (repo_parse_commit(r, commit))\n> +\tif (!commit || repo_parse_commit(r, commit))\n> \t\treturn MISSING_OBJECT;\n\nSure, but repo_parse_commit() can also handle NULL values. It returns\n\"-1\" in that case. I think CodeQL is not smart enough to know that.\n\n-Peff\n"}]}