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
