refs/files: simplify iterating through root refs

When iterating through root refs we first need to determine the
directory in which the refs live. This is done by retrieving the root of
the loose refs via `refs->loose->root->name`, and putting it through
`files_ref_path()` to derive the final path.

This is somewhat redundant though: the root name of the loose files
cache is always going to be the empty string. As such, we always end up
passing that empty string to `files_ref_path()` as the ref hierarchy we
want to start. And this actually makes sense: `files_ref_path()` already
computes the location of the root directory, so of course we need to
pass the empty string for the ref hierarchy itself. So going via the
loose ref cache to figure out that the root of a ref hierarchy is empty
is only causing confusion.

But next to the added confusion, it can also lead to a segfault. The
loose ref cache is populated lazily, so it may not always be set. It
seems to be sheer luck that this is a condition we do not currently hit.
The right thing to do would be to call `get_loose_ref_cache()`, which
knows to populate the cache if required.

Simplify the code and fix the potential segfault by simply removing the
indirection via the loose ref cache completely.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 6f6f76a8d86dc47bc7488df18374971f69306619..297739f203d74e18f5e11517dd484490e6da6918 100644 (file)
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -354,13 +354,11 @@ static int for_each_root_ref(struct files_ref_store *refs,
                             void *cb_data)
 {
        struct strbuf path = STRBUF_INIT, refname = STRBUF_INIT;
-       const char *dirname = refs->loose->root->name;
        struct dirent *de;
-       size_t dirnamelen;
        int ret;
        DIR *d;
 
-       files_ref_path(refs, &path, dirname);
+       files_ref_path(refs, &path, "");
 
        d = opendir(path.buf);
        if (!d) {
@@ -368,9 +366,6 @@ static int for_each_root_ref(struct files_ref_store *refs,
                return -1;
        }
 
-       strbuf_addstr(&refname, dirname);
-       dirnamelen = refname.len;
-
        while ((de = readdir(d)) != NULL) {
                unsigned char dtype;
 
@@ -378,6 +373,8 @@ static int for_each_root_ref(struct files_ref_store *refs,
                        continue;
                if (ends_with(de->d_name, ".lock"))
                        continue;
+
+               strbuf_reset(&refname);
                strbuf_addstr(&refname, de->d_name);
 
                dtype = get_dtype(de, &path, 1);
@@ -386,8 +383,6 @@ static int for_each_root_ref(struct files_ref_store *refs,
                        if (ret)
                                goto done;
                }
-
-               strbuf_setlen(&refname, dirnamelen);
        }
 
        ret = 0;