From: Jeff King <peff@peff•net>
To: Junio C Hamano <gitster@pobox•com>
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 3/5] describe: catch unborn branch in describe_blob()
Date: Mon, 18 Aug 2025 19:07:17 -0400 [thread overview]
Message-ID: <20250818230717.GA1027509@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqy0rggwep.fsf@gitster.g>
On Mon, Aug 18, 2025 at 02:19:42PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff•net> writes:
>
> > --- a/builtin/describe.c
> > +++ b/builtin/describe.c
> > @@ -518,10 +518,16 @@ 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};
>
> An unrelated tangent, but it seems that we are copying the object
> name for the first member of this struct, even though 1/5 changed
> the second one.
Yep, agreed. It goes away in the final patch, though.
> > + if (repo_get_oid(the_repository, "HEAD", &head_oid))
> > + die(_("cannot search for blob '%s' on an unborn branch"),
> > + oid_to_hex(oid));
>
> Makes sense. I briefly wondered if there is really a point in doing
> the traversal only from HEAD, but this topic is not about enhancing
> and making the "describe <blob>" more useful, but it is a strict
> improvement.
Yeah. We do document the traversal from HEAD, so I wondered if anybody
would be upset at being more inclusive. So it might need a new option.
Somebody who is more interested in the option is welcome to pick up that
topic. :)
> When you first mentioned "resolve HEAD ourselves", I somehow expected
> you to ask the ref subsystem to resolve HEAD, but this should do fine,
> thanks to ref_rev_parse_rules[].
I hadn't really thought about the distinction. If you grep for '"HEAD"',
looks like we have a mix of both types. I don't think it should matter
much in practice.
Some even call lookup_commit_reference_by_name(). So we could perhaps do
that, in which case the segfault fix in patch 4 just happens naturally,
because we know we are always traversing from at least one commit. I
don't see any difference between the approaches compelling enough to
switch, though.
-Peff
next prev parent reply other threads:[~2025-08-18 23:07 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 [this message]
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
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=20250818230717.GA1027509@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--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