[PATCH 5/5] describe: pass commit to describe_commit()
- From
Jeff King <peff@peff.net>
- Date
- Aug 18, 2025, 21:04 UTC
- Message-ID
- <20250818210417.GE1024556@coredump.intra.peff.net>
- 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