From: Jeff King <peff@peff•net>
To: "René Scharfe" <l.s.r@web•de>
Cc: phillip.wood@dunelm•org.uk, Cheng <prophecheng@stu•pku.edu.cn>,
git@vger•kernel.org
Subject: [PATCH 5/5] describe: pass commit to describe_commit()
Date: Mon, 18 Aug 2025 17:04:17 -0400 [thread overview]
Message-ID: <20250818210417.GE1024556@coredump.intra.peff.net> (raw)
In-Reply-To: <20250818205812.GA1018043@coredump.intra.peff.net>
There's a call in describe_commit() to lookup_commit_reference(), but we
don't check the return value. If it returns NULL, we'll segfault as we
immediately dereference the result.
In practice this can never happen, since all callers pass an oid which
came from a "struct commit" already. So we can make this more obvious
by just taking that commit struct in the first place.
Reported-by: Cheng <prophecheng@stu•pku.edu.cn>
Signed-off-by: Jeff King <peff@peff•net>
---
builtin/describe.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 72b2e1162c..04df89d56b 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -313,26 +313,24 @@ static void append_suffix(int depth, const struct object_id *oid, struct strbuf
repo_find_unique_abbrev(the_repository, oid, abbrev));
}
-static void describe_commit(struct object_id *oid, struct strbuf *dst)
+static void describe_commit(struct commit *cmit, struct strbuf *dst)
{
- struct commit *cmit, *gave_up_on = NULL;
+ struct commit *gave_up_on = NULL;
struct commit_list *list;
struct commit_name *n;
struct possible_tag all_matches[MAX_TAGS];
unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
unsigned long seen_commits = 0;
unsigned int unannotated_cnt = 0;
- cmit = lookup_commit_reference(the_repository, oid);
-
n = find_commit_name(&cmit->object.oid);
if (n && (tags || all || n->prio == 2)) {
/*
* Exact match to an existing ref.
*/
append_name(n, dst);
if (n->misnamed || longformat)
- append_suffix(0, n->tag ? get_tagged_oid(n->tag) : oid, dst);
+ append_suffix(0, n->tag ? get_tagged_oid(n->tag) : &cmit->object.oid, dst);
if (suffix)
strbuf_addstr(dst, suffix);
return;
@@ -489,7 +487,7 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)
}
struct process_commit_data {
- struct object_id current_commit;
+ struct commit *current_commit;
const struct object_id *looking_for;
struct strbuf *dst;
struct rev_info *revs;
@@ -498,7 +496,7 @@ struct process_commit_data {
static void process_commit(struct commit *commit, void *data)
{
struct process_commit_data *pcd = data;
- pcd->current_commit = commit->object.oid;
+ pcd->current_commit = commit;
}
static void process_object(struct object *obj, const char *path, void *data)
@@ -507,8 +505,8 @@ static void process_object(struct object *obj, const char *path, void *data)
if (oideq(pcd->looking_for, &obj->oid) && !pcd->dst->len) {
reset_revision_walk();
- if (!is_null_oid(&pcd->current_commit)) {
- describe_commit(&pcd->current_commit, pcd->dst);
+ if (pcd->current_commit) {
+ describe_commit(pcd->current_commit, pcd->dst);
strbuf_addf(pcd->dst, ":%s", path);
}
free_commit_list(pcd->revs->commits);
@@ -521,7 +519,7 @@ static void describe_blob(const struct object_id *oid, struct strbuf *dst)
struct rev_info revs;
struct strvec args = STRVEC_INIT;
struct object_id head_oid;
- struct process_commit_data pcd = { *null_oid(the_hash_algo), oid, dst, &revs};
+ struct process_commit_data pcd = { NULL, oid, dst, &revs};
if (repo_get_oid(the_repository, "HEAD", &head_oid))
die(_("cannot search for blob '%s' on an unborn branch"),
@@ -562,7 +560,7 @@ static void describe(const char *arg, int last_one)
cmit = lookup_commit_reference_gently(the_repository, &oid, 1);
if (cmit)
- describe_commit(&oid, &sb);
+ describe_commit(cmit, &sb);
else if (odb_read_object_info(the_repository->objects,
&oid, NULL) == OBJ_BLOB)
describe_blob(&oid, &sb);
--
2.51.0.326.gecbb38d78e
next prev parent reply other threads:[~2025-08-18 21:04 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-13 0:23 Potential Null Pointer Dereference detected by static analysis tool Cheng
2025-08-13 13:19 ` Phillip Wood
2025-08-14 23:26 ` Jeff King
2025-08-15 15:49 ` Phillip Wood
2025-08-17 9:27 ` René Scharfe
2025-08-18 4:48 ` Jeff King
2025-08-18 5:05 ` Jeff King
2025-08-18 19:56 ` René Scharfe
2025-08-18 20:21 ` Jeff King
2025-08-18 20:56 ` Jeff King
2025-08-18 20:58 ` [PATCH 0/5] fix segfault and other oddities describing blobs Jeff King
2025-08-18 20:59 ` [PATCH 1/5] describe: pass oid struct by const pointer Jeff King
2025-08-18 21:05 ` Junio C Hamano
2025-08-18 21:01 ` [PATCH 2/5] describe: error if blob not found Jeff King
2025-08-18 21:12 ` Junio C Hamano
2025-08-19 8:05 ` Patrick Steinhardt
2025-08-19 18:32 ` René Scharfe
2025-08-18 21:01 ` [PATCH 3/5] describe: catch unborn branch in describe_blob() Jeff King
2025-08-18 21:19 ` Junio C Hamano
2025-08-18 23:07 ` Jeff King
2025-08-18 21:03 ` [PATCH 4/5] describe: handle blob traversal with no commits Jeff King
2025-08-19 8:05 ` Patrick Steinhardt
2025-08-19 16:59 ` Jeff King
2025-08-20 4:34 ` Patrick Steinhardt
2025-08-20 6:30 ` [replacement PATCH " Jeff King
2025-08-18 21:04 ` Jeff King [this message]
2025-08-19 8:05 ` [PATCH 5/5] describe: pass commit to describe_commit() Patrick Steinhardt
2025-08-19 17:02 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250818210417.GE1024556@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=git@vger$(echo .)kernel.org \
--cc=l.s.r@web$(echo .)de \
--cc=phillip.wood@dunelm$(echo .)org.uk \
--cc=prophecheng@stu$(echo .)pku.edu.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox