Similar to the preceding commit, we have effectively given tracking
memory ownership of submodule gitfile paths. Refactor the code to start
tracking allocated strings in a separate `struct strvec` such that we
can easily plug those leaks. Mark now-passing tests as leak free.
Note that ideally, we wouldn't require two separate data structures to
track those paths. But we do need to store `NULL` pointers for the
gitfile paths such that we can indicate that its corresponding entries
in the other arrays do not have such a path at all. And given that
`struct strvec`s cannot store `NULL` pointers we cannot use them to
store this information.
There is another small gotcha that is easy to miss: you may be wondering
why we don't want to store `SUBMODULE_WITH_GITDIR` in the strvec. This
is because this is a mere sentinel value and not actually a string at
all.
Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
---
builtin/mv.c | 44 +++++++++++++----------
t/t4059-diff-submodule-not-initialized.sh | 1 +
t/t7001-mv.sh | 2 ++
t/t7417-submodule-path-url.sh | 1 +
t/t7421-submodule-summary-add.sh | 1 +
5 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index e461d29ca1..81ca910de6 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -82,21 +82,23 @@ static char *add_slash(const char *path)
#define SUBMODULE_WITH_GITDIR ((const char *)1)
-static void prepare_move_submodule(const char *src, int first,
- const char **submodule_gitfile)
+static const char *submodule_gitfile_path(const char *src, int first)
{
struct strbuf submodule_dotgit = STRBUF_INIT;
+ const char *path;
+
if (!S_ISGITLINK(the_repository->index->cache[first]->ce_mode))
die(_("Directory %s is in index and no submodule?"), src);
if (!is_staging_gitmodules_ok(the_repository->index))
die(_("Please stage your changes to .gitmodules or stash them to proceed"));
+
strbuf_addf(&submodule_dotgit, "%s/.git", src);
- *submodule_gitfile = read_gitfile(submodule_dotgit.buf);
- if (*submodule_gitfile)
- *submodule_gitfile = xstrdup(*submodule_gitfile);
- else
- *submodule_gitfile = SUBMODULE_WITH_GITDIR;
+
+ path = read_gitfile(submodule_dotgit.buf);
strbuf_release(&submodule_dotgit);
+ if (path)
+ return path;
+ return SUBMODULE_WITH_GITDIR;
}
static int index_range_of_same_dir(const char *src, int length,
@@ -170,7 +172,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
struct strvec sources = STRVEC_INIT;
struct strvec dest_paths = STRVEC_INIT;
struct strvec destinations = STRVEC_INIT;
- const char **submodule_gitfile;
+ struct strvec submodule_gitfiles_to_free = STRVEC_INIT;
+ const char **submodule_gitfiles;
char *dst_w_slash = NULL;
const char **src_dir = NULL;
int src_dir_nr = 0, src_dir_alloc = 0;
@@ -208,7 +211,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
flags = 0;
internal_prefix_pathspec(&dest_paths, prefix, argv + argc, 1, flags);
dst_w_slash = add_slash(dest_paths.v[0]);
- submodule_gitfile = xcalloc(argc, sizeof(char *));
+ submodule_gitfiles = xcalloc(argc, sizeof(char *));
if (dest_paths.v[0][0] == '\0')
/* special case: "." was normalized to "" */
@@ -306,8 +309,10 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
int first = index_name_pos(the_repository->index, src, length), last;
if (first >= 0) {
- prepare_move_submodule(src, first,
- submodule_gitfile + i);
+ const char *path = submodule_gitfile_path(src, first);
+ if (path != SUBMODULE_WITH_GITDIR)
+ path = strvec_push(&submodule_gitfiles_to_free, path);
+ submodule_gitfiles[i] = path;
goto act_on_entry;
} else if (index_range_of_same_dir(src, length,
&first, &last) < 1) {
@@ -323,7 +328,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
n = argc + last - first;
REALLOC_ARRAY(modes, n);
- REALLOC_ARRAY(submodule_gitfile, n);
+ REALLOC_ARRAY(submodule_gitfiles, n);
dst_with_slash = add_slash(dst);
dst_with_slash_len = strlen(dst_with_slash);
@@ -338,7 +343,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
memset(modes + argc + j, 0, sizeof(enum update_mode));
modes[argc + j] |= ce_skip_worktree(ce) ? SPARSE : INDEX;
- submodule_gitfile[argc + j] = NULL;
+ submodule_gitfiles[argc + j] = NULL;
free(prefixed_path);
}
@@ -427,8 +432,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
strvec_remove(&sources, i);
strvec_remove(&destinations, i);
MOVE_ARRAY(modes + i, modes + i + 1, n);
- MOVE_ARRAY(submodule_gitfile + i,
- submodule_gitfile + i + 1, n);
+ MOVE_ARRAY(submodule_gitfiles + i,
+ submodule_gitfiles + i + 1, n);
i--;
}
}
@@ -462,12 +467,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
continue;
die_errno(_("renaming '%s' failed"), src);
}
- if (submodule_gitfile[i]) {
+ if (submodule_gitfiles[i]) {
if (!update_path_in_gitmodules(src, dst))
gitmodules_modified = 1;
- if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
+ if (submodule_gitfiles[i] != SUBMODULE_WITH_GITDIR)
connect_work_tree_and_git_dir(dst,
- submodule_gitfile[i],
+ submodule_gitfiles[i],
1);
}
@@ -573,7 +578,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
strvec_clear(&sources);
strvec_clear(&dest_paths);
strvec_clear(&destinations);
- free(submodule_gitfile);
+ strvec_clear(&submodule_gitfiles_to_free);
+ free(submodule_gitfiles);
free(modes);
return ret;
}
diff --git a/t/t4059-diff-submodule-not-initialized.sh b/t/t4059-diff-submodule-not-initialized.sh
index d489230df8..668f526303 100755
--- a/t/t4059-diff-submodule-not-initialized.sh
+++ b/t/t4059-diff-submodule-not-initialized.sh
@@ -9,6 +9,7 @@ This test tries to verify that add_submodule_odb works when the submodule was
initialized previously but the checkout has since been removed.
'
+TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
# Tested non-UTF-8 encoding
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 879a6dce60..86258f9f43 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -1,6 +1,8 @@
#!/bin/sh
test_description='git mv in subdirs'
+
+TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-diff-data.sh
diff --git a/t/t7417-submodule-path-url.sh b/t/t7417-submodule-path-url.sh
index 5e3051da8b..dbbb3853dc 100755
--- a/t/t7417-submodule-path-url.sh
+++ b/t/t7417-submodule-path-url.sh
@@ -4,6 +4,7 @@ test_description='check handling of .gitmodule path with dash'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success 'setup' '
diff --git a/t/t7421-submodule-summary-add.sh b/t/t7421-submodule-summary-add.sh
index ce64d8b137..479c8fdde1 100755
--- a/t/t7421-submodule-summary-add.sh
+++ b/t/t7421-submodule-summary-add.sh
@@ -10,6 +10,7 @@ while making sure to add submodules using `git submodule add` instead of
`git add` as done in t7401.
'
+TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success 'setup' '
--
2.45.1.216.g4365c6fcf9.dirty
Attachment:
signature.asc
Description: PGP signature
