Skip to content

Commit 0220461

Browse files
peffgitster
authored andcommitted
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 5ff4b92 (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 5ff4b92, 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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 5ff4b92 commit 0220461

File tree

3 files changed

+11
-7
lines changed

3 files changed

+11
-7
lines changed

refs.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1774,14 +1774,14 @@ static struct ref_store *ref_store_init(const char *gitdir,
17741774

17751775
struct ref_store *get_main_ref_store(struct repository *r)
17761776
{
1777-
if (r->refs)
1778-
return r->refs;
1777+
if (r->refs_private)
1778+
return r->refs_private;
17791779

17801780
if (!r->gitdir)
17811781
BUG("attempting to get main_ref_store outside of repository");
17821782

1783-
r->refs = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
1784-
return r->refs;
1783+
r->refs_private = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
1784+
return r->refs_private;
17851785
}
17861786

17871787
/*

repository.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,12 @@ struct repository {
3939
*/
4040
struct parsed_object_pool *parsed_objects;
4141

42-
/* The store in which the refs are held. */
43-
struct ref_store *refs;
42+
/*
43+
* The store in which the refs are held. This should generally only be
44+
* accessed via get_main_ref_store(), as that will lazily initialize
45+
* the ref object.
46+
*/
47+
struct ref_store *refs_private;
4448

4549
/*
4650
* Contains path to often used file names.

sha1-name.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1772,7 +1772,7 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
17721772
cb.repo = repo;
17731773
cb.list = &list;
17741774
refs_for_each_ref(get_main_ref_store(repo), handle_one_ref, &cb);
1775-
refs_head_ref(repo->refs, handle_one_ref, &cb);
1775+
refs_head_ref(get_main_ref_store(repo), handle_one_ref, &cb);
17761776
commit_list_sort_by_date(&list);
17771777
return get_oid_oneline(repo, name + 2, oid, list);
17781778
}

0 commit comments

Comments
 (0)