builtin/mv: fix leaks for submodule gitfile paths
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@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
May 27, 2024 at 13:47 UTC
ebdbefa4fe9f618347124b37d44e517e0c6a3e4c
5 files changed
+30
-19
builtin/mv.c
+25
-19
@@ -82,21 +82,23 @@ static char *add_slash(const char *path)
82
83
#define SUBMODULE_WITH_GITDIR ((const char *)1)
84
85
-static void prepare_move_submodule(const char *src, int first,
86
- const char **submodule_gitfile)
85
+static const char *submodule_gitfile_path(const char *src, int first)
86
{
87
struct strbuf submodule_dotgit = STRBUF_INIT;
88
+ const char *path;
89
+
90
if (!S_ISGITLINK(the_repository->index->cache[first]->ce_mode))
91
die(_("Directory %s is in index and no submodule?"), src);
92
if (!is_staging_gitmodules_ok(the_repository->index))
93
die(_("Please stage your changes to .gitmodules or stash them to proceed"));
94
+
95
strbuf_addf(&submodule_dotgit, "%s/.git", src);
94
- *submodule_gitfile = read_gitfile(submodule_dotgit.buf);
95
- if (*submodule_gitfile)
96
- *submodule_gitfile = xstrdup(*submodule_gitfile);
97
- else
98
- *submodule_gitfile = SUBMODULE_WITH_GITDIR;
96
+
97
+ path = read_gitfile(submodule_dotgit.buf);
98
strbuf_release(&submodule_dotgit);
99
+ if (path)
100
+ return path;
101
+ return SUBMODULE_WITH_GITDIR;
102
}
103
104
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)
172
struct strvec sources = STRVEC_INIT;
173
struct strvec dest_paths = STRVEC_INIT;
174
struct strvec destinations = STRVEC_INIT;
173
- const char **submodule_gitfile;
175
+ struct strvec submodule_gitfiles_to_free = STRVEC_INIT;
176
+ const char **submodule_gitfiles;
177
char *dst_w_slash = NULL;
178
const char **src_dir = NULL;
179
int src_dir_nr = 0, src_dir_alloc = 0;
@@ -208,7 +211,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
211
flags = 0;
212
internal_prefix_pathspec(&dest_paths, prefix, argv + argc, 1, flags);
213
dst_w_slash = add_slash(dest_paths.v[0]);
211
- submodule_gitfile = xcalloc(argc, sizeof(char *));
214
+ submodule_gitfiles = xcalloc(argc, sizeof(char *));
215
216
if (dest_paths.v[0][0] == '\0')
217
/* special case: "." was normalized to "" */
@@ -306,8 +309,10 @@ dir_check:
309
int first = index_name_pos(the_repository->index, src, length), last;
310
311
if (first >= 0) {
309
- prepare_move_submodule(src, first,
310
- submodule_gitfile + i);
312
+ const char *path = submodule_gitfile_path(src, first);
313
+ if (path != SUBMODULE_WITH_GITDIR)
314
+ path = strvec_push(&submodule_gitfiles_to_free, path);
315
+ submodule_gitfiles[i] = path;
316
goto act_on_entry;
317
} else if (index_range_of_same_dir(src, length,
318
&first, &last) < 1) {
@@ -323,7 +328,7 @@ dir_check:
328
329
n = argc + last - first;
330
REALLOC_ARRAY(modes, n);
326
- REALLOC_ARRAY(submodule_gitfile, n);
331
+ REALLOC_ARRAY(submodule_gitfiles, n);
332
333
dst_with_slash = add_slash(dst);
334
dst_with_slash_len = strlen(dst_with_slash);
@@ -338,7 +343,7 @@ dir_check:
343
344
memset(modes + argc + j, 0, sizeof(enum update_mode));
345
modes[argc + j] |= ce_skip_worktree(ce) ? SPARSE : INDEX;
341
- submodule_gitfile[argc + j] = NULL;
346
+ submodule_gitfiles[argc + j] = NULL;
347
348
free(prefixed_path);
349
}
@@ -427,8 +432,8 @@ remove_entry:
432
strvec_remove(&sources, i);
433
strvec_remove(&destinations, i);
434
MOVE_ARRAY(modes + i, modes + i + 1, n);
430
- MOVE_ARRAY(submodule_gitfile + i,
431
- submodule_gitfile + i + 1, n);
435
+ MOVE_ARRAY(submodule_gitfiles + i,
436
+ submodule_gitfiles + i + 1, n);
437
i--;
438
}
439
}
@@ -462,12 +467,12 @@ remove_entry:
467
continue;
468
die_errno(_("renaming '%s' failed"), src);
469
}
465
- if (submodule_gitfile[i]) {
470
+ if (submodule_gitfiles[i]) {
471
if (!update_path_in_gitmodules(src, dst))
472
gitmodules_modified = 1;
468
- if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
473
+ if (submodule_gitfiles[i] != SUBMODULE_WITH_GITDIR)
474
connect_work_tree_and_git_dir(dst,
470
- submodule_gitfile[i],
475
+ submodule_gitfiles[i],
476
1);
477
}
478
@@ -573,7 +578,8 @@ out:
578
strvec_clear(&sources);
579
strvec_clear(&dest_paths);
580
strvec_clear(&destinations);
576
- free(submodule_gitfile);
581
+ strvec_clear(&submodule_gitfiles_to_free);
582
+ free(submodule_gitfiles);
583
free(modes);
584
return ret;
585
}
t/t4059-diff-submodule-not-initialized.sh
+1
@@ -9,6 +9,7 @@ This test tries to verify that add_submodule_odb works when the submodule was
9
initialized previously but the checkout has since been removed.
10
'
11
12
+TEST_PASSES_SANITIZE_LEAK=true
13
. ./test-lib.sh
14
15
# Tested non-UTF-8 encoding
t/t7001-mv.sh
+2
@@ -1,6 +1,8 @@
1
#!/bin/sh
2
3
test_description='git mv in subdirs'
4
+
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
. "$TEST_DIRECTORY"/lib-diff-data.sh
8
t/t7417-submodule-path-url.sh
+1
@@ -4,6 +4,7 @@ test_description='check handling of .gitmodule path with dash'
4
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
5
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
6
7
+TEST_PASSES_SANITIZE_LEAK=true
8
. ./test-lib.sh
9
10
test_expect_success 'setup' '
t/t7421-submodule-summary-add.sh
+1
@@ -10,6 +10,7 @@ while making sure to add submodules using `git submodule add` instead of
10
`git add` as done in t7401.
11
'
12
13
+TEST_PASSES_SANITIZE_LEAK=true
14
. ./test-lib.sh
15
16
test_expect_success 'setup' '