From 380f864a10aeadd30bd88138906d4fab577221de Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 26 Mar 2014 16:06:21 -0700 Subject: [PATCH] Fix error when submodule path and name differ When a submodule was inserted with a different path and name, the return value from khash greater than zero was allowed to propagate back out to the caller when it should really be zeroed. This led to a possible crash when reloading submodules if that was the first time that submodule data was loaded. --- src/submodule.c | 7 +++++-- tests/submodule/lookup.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/submodule.c b/src/submodule.c index 54ffc6103..b07c9d917 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -1279,14 +1279,17 @@ static int submodule_load_from_config( } } + /* Found a alternate key for the submodule */ if (alternate) { void *old_sm = NULL; git_strmap_insert2(smcfg, alternate, sm, old_sm, error); if (error < 0) goto done; - if (error >= 0) - GIT_REFCOUNT_INC(sm); /* inserted under a new key */ + if (error > 0) + error = 0; + + GIT_REFCOUNT_INC(sm); /* increase refcount for new key */ /* if we replaced an old module under this key, release the old one */ if (old_sm && ((git_submodule *)old_sm) != sm) { diff --git a/tests/submodule/lookup.c b/tests/submodule/lookup.c index cc29b11b2..36bde4f6e 100644 --- a/tests/submodule/lookup.c +++ b/tests/submodule/lookup.c @@ -141,3 +141,36 @@ void test_submodule_lookup__lookup_even_with_missing_index(void) refute_submodule_exists(g_repo, "just_a_file", GIT_ENOTFOUND); refute_submodule_exists(g_repo, "no_such_file", GIT_ENOTFOUND); } + +void test_submodule_lookup__just_added(void) +{ + git_submodule *sm; + + cl_git_pass(git_submodule_add_setup(&sm, g_repo, "https://github.com/libgit2/libgit2.git", "sm_just_added", 1)); + git_submodule_free(sm); + assert_submodule_exists(g_repo, "sm_just_added"); + + cl_git_pass(git_submodule_add_setup(&sm, g_repo, "https://github.com/libgit2/libgit2.git", "sm_just_added_2", 1)); + assert_submodule_exists(g_repo, "sm_just_added_2"); + git_submodule_free(sm); + + cl_git_append2file("submod2/.gitmodules", "\n[submodule \"mismatch_name\"]\n\tpath = mismatch_path\n\turl = https://example.com/example.git\n\n"); + + cl_git_pass(git_submodule_reload_all(g_repo, 1)); + + assert_submodule_exists(g_repo, "mismatch_name"); + assert_submodule_exists(g_repo, "mismatch_path"); + + assert_submodule_exists(g_repo, "sm_just_added"); + assert_submodule_exists(g_repo, "sm_just_added_2"); + + /* all the regular ones should still be working right, too */ + + assert_submodule_exists(g_repo, "sm_unchanged"); + assert_submodule_exists(g_repo, "sm_added_and_uncommited"); + assert_submodule_exists(g_repo, "sm_gitmodules_only"); + refute_submodule_exists(g_repo, "not-submodule", GIT_EEXISTS); + refute_submodule_exists(g_repo, "just_a_dir", GIT_ENOTFOUND); + refute_submodule_exists(g_repo, "just_a_file", GIT_ENOTFOUND); + refute_submodule_exists(g_repo, "no_such_file", GIT_ENOTFOUND); +}