get_oid: handle NULL repo->index

When get_oid() and its helpers see an index name like ":.gitmodules",
they try to load the index on demand, like:

if (repo->index->cache)
repo_read_index(repo);

However, that misses the case when "repo->index" itself is NULL; we'll
segfault in the conditional.

This never happens with the_repository; there we always point its index
field to &the_index. But a submodule repository may have a NULL index
field until somebody calls repo_read_index().

This bug is triggered by t7411, but it was hard to notice because it's
in an expect_failure block. That test was added by 2b1257e463 (t/helper:
add test-submodule-nested-repo-config, 2018-10-25). Back then we had no
easy way to access the .gitmodules blob of a submodule repo, so we
expected (and got) an error message to that effect. Later, d9b8b8f896
(submodule-config.c: use repo_get_oid for reading .gitmodules,
2019-04-16) started looking in the correct repo, which is when we
started triggering the segfault.

With this fix, the test starts passing (once we clean it up as its
comment instructs).

Note that as far as I know, this bug could not be triggered outside of
the test suite. It requires resolving an index name in a submodule, and
all of the code paths (aside from test-tool) which do that either load
the index themselves, or always pass the_repository.

Ultimately it comes from 3a7a698e93 (sha1-name.c: remove implicit
dependency on the_index, 2019-01-12), which replaced a check of
"the_index.cache" with "repo->index->cache". So even if there is another
way to trigger it, it wouldn't affect any versions before then.

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

diff --git a/sha1-name.c b/sha1-name.c
index 775a73d8adf6748cd67796a6b13ea1c4843d9f6a..455e9fb1eaa20e781aa63e67a7ffe41525c9ac57 100644 (file)
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -1837,7 +1837,7 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
                if (flags & GET_OID_RECORD_PATH)
                        oc->path = xstrdup(cp);
 
-               if (!repo->index->cache)
+               if (!repo->index || !repo->index->cache)
                        repo_read_index(repo);
                pos = index_name_pos(repo->index, cp, namelen);
                if (pos < 0)

diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index fcc0fb82d8adb15c592056a259b5f8c5c264d9a2..ad28e9388053c4009f4af3121788951f9e79d848 100755 (executable)
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -243,18 +243,14 @@ test_expect_success 'reading nested submodules config' '
        )
 '
 
-# When this test eventually passes, before turning it into
-# test_expect_success, remember to replace the test_i18ngrep below with
-# a "test_must_be_empty warning" to be sure that the warning is actually
-# removed from the code.
-test_expect_failure 'reading nested submodules config when .gitmodules is not in the working tree' '
+test_expect_success 'reading nested submodules config when .gitmodules is not in the working tree' '
        test_when_finished "git -C super/submodule checkout .gitmodules" &&
        (cd super &&
                echo "./nested_submodule" >expect &&
                rm submodule/.gitmodules &&
                test-tool submodule-nested-repo-config \
                        submodule submodule.nested_submodule.url >actual 2>warning &&
-               test_i18ngrep "nested submodules without %s in the working tree are not supported yet" warning &&
+               test_must_be_empty warning &&
                test_cmp expect actual
        )
 '