On Sun, Aug 17, 2025 at 11:27:12AM +0200, René Scharfe wrote:
> > @@ -546,7 +544,7 @@ 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();
> > - describe_commit(&pcd->current_commit, pcd->dst);
> > + describe_commit(pcd->current_commit, pcd->dst);
>
> pcd->current_commit is initialized to NULL below, but
> traverse_commit_list() without a filter must have set it via our
> process_commit() callback before we get to the describe_commit() call.
>
> Or are there weird repositories (e.g., just a blob, just a tag) that can
> cause traverse_commit_list() to call its show_object() callback without
> ever calling its show_commit() callback? I don't see how, but may be
> missing some way.
If there are, then I think the current code would segfault, too. It
initializes &pcd->current_commit to the null oid, and then
describe_commit() resolves that via lookup_commit_reference(). That
would return NULL, and the next line dereferencing the result would
segfault.
And it would be a counter-example to the claim that the call to
lookup_commit_reference() in describe_commit() never fails. ;)
I think your intuition is right that we could get the traversal code to
call show_object() without show_commit() in general. E.g. just:
git init
git tag foo $(echo bar | git hash-object -w --stdin)
git rev-list --all --objects
would do so. But in this code our traversal always starts from HEAD,
which must be a commit (and this is enforced by the refs code). So you'd
have to corrupt your repository like:
# do this in two steps since redirecting to .git/HEAD breaks the
# repository for a moment!
tag=$(git rev-parse foo)
echo $tag >.git/HEAD
And indeed, the current code does then segfault on "git describe foo" at
the spot I mentioned. Even though the repository state here is
unexpected and corrupt, I do think we should probably be more defensive
and avoid the segfault.
I also find it a bit funny that describing a blob only walks from HEAD
(and not say, all refs or ones matching --match/--exclude, etc). It is
documented that way, though. Actually, I find the whole feature a bit
pointless now that we have the diff "--find-object" option. Reading
15af58c1ad (diffcore: add a pickaxe option to find a specific blob,
2018-01-04), I think the git-describe behavior is mostly considered a
mistake (which we have to keep around for historical reasons). I guess
another possible candidate for removing in Git 3.0. :)
-Peff
