On Sun, Aug 17, 2025 at 11:27:12AM +0200, René Scharfe wrote:

quoted

@@ -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