Re: [BUG] segmentation fault in git-diff

[Jeff King <peff@peff.net>]

On Thu, Apr 09, 2020 at 03:57:00PM -0700, Junio C Hamano wrote:

> $ git checkout v2.22.0 && make && ./git-rev-parse :/any/path/
> 
> segfaults, while
> 
> $ git checkout v2.21.0 && make && ./git-rev-parse :/any/path/
> 
> is OK.  We should be able to bisect this fairly straightforward
> between these two.

Indeed. The culprit is c931ba4e78 (sha1-name.c: remove the_repo from
handle_one_ref(), 2019-04-16), which swapped out for_each_ref() for
refs_for_each_ref(repo->refs). But it misses the access method for
repo->refs which lazy-initializes the pointer.

So the immediate fix is:

diff --git a/sha1-name.c b/sha1-name.c
index d9050776dd..c679e246cd 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -1771,8 +1771,8 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
 
 			cb.repo = repo;
 			cb.list = &list;
-			refs_for_each_ref(repo->refs, handle_one_ref, &cb);
-			refs_head_ref(repo->refs, handle_one_ref, &cb);
+			refs_for_each_ref(get_main_ref_store(repo), handle_one_ref, &cb);
+			refs_head_ref(get_main_ref_store(repo), handle_one_ref, &cb);
 			commit_list_sort_by_date(&list);
 			return get_oid_oneline(repo, name + 2, oid, list);
 		}

But there are a bunch of other commits around the same time replacing
the_repository, and it seems like an easy mistake to make. Perhaps we
should rename the "refs" member of "struct repository" to something more
clearly private, which would force callers to use the access method.

I also wonder if there should be a repo_for_each_ref() which does it for
us, though I guess there a bazillion variants (like head_ref()) that
would need similar treatment. Asking each caller to use
get_main_ref_store() isn't too bad.

-Peff