From: Patrick Steinhardt <ps@pks.im>
To: Jeff King <peff@peff.net>
Cc: "René Scharfe" <l.s.r@web.de>,
phillip.wood@dunelm.org.uk, Cheng <prophecheng@stu.pku.edu.cn>,
git@vger.kernel.org
Subject: Re: [PATCH 5/5] describe: pass commit to describe_commit()
Date: Tue, 19 Aug 2025 10:05:25 +0200 [thread overview]
Message-ID: <aKQwRaX94uwTwiQP@pks.im> (raw)
In-Reply-To: <20250818210417.GE1024556@coredump.intra.peff.net>
On Mon, Aug 18, 2025 at 05:04:17PM -0400, Jeff King wrote:
> 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.
I was wondering a bit about commit-graphs. We had the case in the past
where it was possible to look up commits via the graph even though they
don't exist in the ODB. So we might actually end up with a missing
object if `GIT_COMMIT_GRAPH_PARANOIA=false`, which is the default value.
But that might be fine? No idea without digging further.
In any case, the refactoring makes sense regardless from my point of
view.
Patrick
next prev parent reply other threads:[~2025-08-19 8:05 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 ` [PATCH 5/5] describe: pass commit to describe_commit() Jeff King
2025-08-19 8:05 ` Patrick Steinhardt [this message]
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=aKQwRaX94uwTwiQP@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=l.s.r@web.de \
--cc=peff@peff.net \
--cc=phillip.wood@dunelm.org.uk \
--cc=prophecheng@stu.pku.edu.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).