Skip to content

Commit 581d2fd

Browse files
peffgitster
authored andcommitted
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 2b1257e (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, d9b8b8f (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 3a7a698 (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ab15ad1 commit 581d2fd

File tree

2 files changed

+3
-7
lines changed

2 files changed

+3
-7
lines changed

sha1-name.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -1837,7 +1837,7 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
18371837
if (flags & GET_OID_RECORD_PATH)
18381838
oc->path = xstrdup(cp);
18391839

1840-
if (!repo->index->cache)
1840+
if (!repo->index || !repo->index->cache)
18411841
repo_read_index(repo);
18421842
pos = index_name_pos(repo->index, cp, namelen);
18431843
if (pos < 0)

t/t7411-submodule-config.sh

+2-6
Original file line numberDiff line numberDiff line change
@@ -243,18 +243,14 @@ test_expect_success 'reading nested submodules config' '
243243
)
244244
'
245245

246-
# When this test eventually passes, before turning it into
247-
# test_expect_success, remember to replace the test_i18ngrep below with
248-
# a "test_must_be_empty warning" to be sure that the warning is actually
249-
# removed from the code.
250-
test_expect_failure 'reading nested submodules config when .gitmodules is not in the working tree' '
246+
test_expect_success 'reading nested submodules config when .gitmodules is not in the working tree' '
251247
test_when_finished "git -C super/submodule checkout .gitmodules" &&
252248
(cd super &&
253249
echo "./nested_submodule" >expect &&
254250
rm submodule/.gitmodules &&
255251
test-tool submodule-nested-repo-config \
256252
submodule submodule.nested_submodule.url >actual 2>warning &&
257-
test_i18ngrep "nested submodules without %s in the working tree are not supported yet" warning &&
253+
test_must_be_empty warning &&
258254
test_cmp expect actual
259255
)
260256
'

0 commit comments

Comments
 (0)