repository: mark the "refs" pointer as private

The "refs" pointer in a struct repository starts life as NULL, but then
is lazily initialized when it is accessed via get_main_ref_store().
However, it's easy for calling code to forget this and access it
directly, leading to code which works _some_ of the time, but fails if
it is called before anybody else accesses the refs.

This was the cause of the bug fixed by 5ff4b920eb (sha1-name: do not
assume that the ref store is initialized, 2020-04-09). In order to
prevent similar bugs, let's more clearly mark the "refs" field as
private.

In addition to helping future code, the name change will help us audit
any existing direct uses. Besides get_main_ref_store() itself, it turns
out there is only one. But we know it's OK as it is on the line directly
after the fix from 5ff4b920eb, which will have initialized the pointer.
However it's still a good idea for it to model the proper use of the
accessing function, so we'll convert it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs.c b/refs.c
index 92d1f6dbdd0d313cdf109e02f31a51bb55cfc601..4cc32e7aad436aa566ca7641724a53d8a837d930 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -1774,14 +1774,14 @@ static struct ref_store *ref_store_init(const char *gitdir,
 
 struct ref_store *get_main_ref_store(struct repository *r)
 {
-       if (r->refs)
-               return r->refs;
+       if (r->refs_private)
+               return r->refs_private;
 
        if (!r->gitdir)
                BUG("attempting to get main_ref_store outside of repository");
 
-       r->refs = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
-       return r->refs;
+       r->refs_private = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
+       return r->refs_private;
 }
 
 /*

diff --git a/repository.h b/repository.h
index 4fb6a5885f794dea9ff7a1ecf37f27bc34e4d218..4ecc201f89dc94e27d434846740b8d3f618ad494 100644 (file)
--- a/repository.h
+++ b/repository.h
@@ -39,8 +39,12 @@ struct repository {
         */
        struct parsed_object_pool *parsed_objects;
 
-       /* The store in which the refs are held. */
-       struct ref_store *refs;
+       /*
+        * The store in which the refs are held. This should generally only be
+        * accessed via get_main_ref_store(), as that will lazily initialize
+        * the ref object.
+        */
+       struct ref_store *refs_private;
 
        /*
         * Contains path to often used file names.

diff --git a/sha1-name.c b/sha1-name.c
index 3aba62938f9f651f1b959866cfa0658d777e680d..c679e246cdcfc690c25e057fb33187fc10bb21c2 100644 (file)
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -1772,7 +1772,7 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
                        cb.repo = repo;
                        cb.list = &list;
                        refs_for_each_ref(get_main_ref_store(repo), handle_one_ref, &cb);
-                       refs_head_ref(repo->refs, 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);
                }