Merge pull request #4641 from pks-t/pks/submodule-names-memleak Detect duplicated submodules for the same path
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186
diff --git a/src/submodule.c b/src/submodule.c
index b927b17..d0aef2e 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -199,13 +199,16 @@ out:
*/
static void free_submodule_names(git_strmap *names)
{
- git_buf *name = 0;
+ git_buf *name;
+
if (names == NULL)
return;
+
git_strmap_foreach_value(names, name, {
git__free(name);
});
git_strmap_free(names);
+
return;
}
@@ -214,23 +217,36 @@ static void free_submodule_names(git_strmap *names)
* TODO: for some use-cases, this might need case-folding on a
* case-insensitive filesystem
*/
-static int load_submodule_names(git_strmap *out, git_repository *repo, git_config *cfg)
+static int load_submodule_names(git_strmap **out, git_repository *repo, git_config *cfg)
{
const char *key = "submodule\\..*\\.path";
- git_config_iterator *iter;
+ git_config_iterator *iter = NULL;
git_config_entry *entry;
git_buf buf = GIT_BUF_INIT;
+ git_strmap *names;
int rval, isvalid;
int error = 0;
+ *out = NULL;
+
+ if ((error = git_strmap_alloc(&names)) < 0)
+ goto out;
+
if ((error = git_config_iterator_glob_new(&iter, cfg, key)) < 0)
- return error;
+ goto out;
while (git_config_next(&entry, iter) == 0) {
const char *fdot, *ldot;
fdot = strchr(entry->name, '.');
ldot = strrchr(entry->name, '.');
+ if (git_strmap_exists(names, entry->value)) {
+ giterr_set(GITERR_SUBMODULE,
+ "duplicated submodule path '%s'", entry->value);
+ error = -1;
+ goto out;
+ }
+
git_buf_clear(&buf);
git_buf_put(&buf, fdot + 1, ldot - fdot - 1);
isvalid = git_submodule_name_is_valid(repo, buf.ptr, 0);
@@ -241,7 +257,7 @@ static int load_submodule_names(git_strmap *out, git_repository *repo, git_confi
if (!isvalid)
continue;
- git_strmap_insert(out, entry->value, git_buf_detach(&buf), &rval);
+ git_strmap_insert(names, entry->value, git_buf_detach(&buf), &rval);
if (rval < 0) {
giterr_set(GITERR_NOMEMORY, "error inserting submodule into hash table");
return -1;
@@ -250,7 +266,11 @@ static int load_submodule_names(git_strmap *out, git_repository *repo, git_confi
if (error == GIT_ITEROVER)
error = 0;
+ *out = names;
+ names = NULL;
+
out:
+ free_submodule_names(names);
git_buf_free(&buf);
git_config_iterator_free(iter);
return error;
@@ -436,10 +456,9 @@ static int submodules_from_index(git_strmap *map, git_index *idx, git_config *cf
int error;
git_iterator *i = NULL;
const git_index_entry *entry;
- git_strmap *names = 0;
+ git_strmap *names;
- git_strmap_alloc(&names);
- if ((error = load_submodule_names(names, git_index_owner(idx), cfg)))
+ if ((error = load_submodule_names(&names, git_index_owner(idx), cfg)))
goto done;
if ((error = git_iterator_for_index(&i, git_index_owner(idx), idx, NULL)) < 0)
@@ -489,9 +508,9 @@ static int submodules_from_head(git_strmap *map, git_tree *head, git_config *cfg
int error;
git_iterator *i = NULL;
const git_index_entry *entry;
- git_strmap *names = 0;
- git_strmap_alloc(&names);
- if ((error = load_submodule_names(names, git_tree_owner(head), cfg)))
+ git_strmap *names;
+
+ if ((error = load_submodule_names(&names, git_tree_owner(head), cfg)))
goto done;
if ((error = git_iterator_for_tree(&i, head, NULL)) < 0)
@@ -553,7 +572,6 @@ int git_submodule__map(git_repository *repo, git_strmap *map)
git_buf path = GIT_BUF_INIT;
git_submodule *sm;
git_config *mods = NULL;
- uint32_t mask;
assert(repo && map);
@@ -567,22 +585,6 @@ int git_submodule__map(git_repository *repo, git_strmap *map)
if (wd && (error = git_buf_joinpath(&path, wd, GIT_MODULES_FILE)) < 0)
goto cleanup;
- /* clear submodule flags that are to be refreshed */
- mask = 0;
- mask |= GIT_SUBMODULE_STATUS_IN_INDEX |
- GIT_SUBMODULE_STATUS__INDEX_FLAGS |
- GIT_SUBMODULE_STATUS__INDEX_OID_VALID |
- GIT_SUBMODULE_STATUS__INDEX_MULTIPLE_ENTRIES;
-
- mask |= GIT_SUBMODULE_STATUS_IN_HEAD |
- GIT_SUBMODULE_STATUS__HEAD_OID_VALID;
- mask |= GIT_SUBMODULE_STATUS_IN_CONFIG;
- if (mask != 0)
- mask |= GIT_SUBMODULE_STATUS_IN_WD |
- GIT_SUBMODULE_STATUS__WD_SCANNED |
- GIT_SUBMODULE_STATUS__WD_FLAGS |
- GIT_SUBMODULE_STATUS__WD_OID_VALID;
-
/* add submodule information from .gitmodules */
if (wd) {
lfc_data data = { 0 };
@@ -611,7 +613,7 @@ int git_submodule__map(git_repository *repo, git_strmap *map)
goto cleanup;
}
/* shallow scan submodules in work tree as needed */
- if (wd && mask != 0) {
+ if (wd) {
git_strmap_foreach_value(map, sm, {
submodule_load_from_wd_lite(sm);
});
diff --git a/tests/submodule/lookup.c b/tests/submodule/lookup.c
index 170be5a..5db5c2d 100644
--- a/tests/submodule/lookup.c
+++ b/tests/submodule/lookup.c
@@ -132,6 +132,32 @@ void test_submodule_lookup__foreach(void)
cl_assert_equal_i(8, data.count);
}
+static int sm_dummy_cb(git_submodule *sm, const char *name, void *payload)
+{
+ GIT_UNUSED(sm);
+ GIT_UNUSED(name);
+ GIT_UNUSED(payload);
+ return 0;
+}
+
+void test_submodule_lookup__duplicated_path(void)
+{
+ /*
+ * Manually invoke cleanup methods to remove leftovers
+ * from `setup_fixture_submod2`
+ */
+ cl_git_sandbox_cleanup();
+ cl_fixture_cleanup("submod2_target");
+
+ g_repo = setup_fixture_submodules();
+
+ /*
+ * This should fail, as the submodules repo has an
+ * invalid gitmodules file with duplicated paths.
+ */
+ cl_git_fail(git_submodule_foreach(g_repo, sm_dummy_cb, NULL));
+}
+
void test_submodule_lookup__lookup_even_with_unborn_head(void)
{
git_reference *head;