@matheusbsbs / matheus-bagliano / commits / e77b3da6bb

submodule--helper: fix a leak in "clone_submodule"

Fix a memory leak of the "clone_data_path" variable that we copy or derive from the "struct module_clone_data" in clone_submodule(). This code was refactored in preceding commits, but the leak has been with us since f8eaa0ba98b (submodule--helper, module_clone: always operate on absolute paths, 2016-03-31). For the "else" case we don't need to xstrdup() the "clone_data->path", and we don't need to free our own "clone_data_path". We can therefore assign the "clone_data->path" to our own "clone_data_path" right away, and only override it (and remember to free it!) if we need to xstrfmt() a replacement. In the case of the module_clone() caller it's from "argv", and doesn't need to be free'd, and in the case of the add_submodule() caller we get a pointer to "sm_path", which doesn't need to be directly free'd either. Fixing this leak makes several tests pass, so let's mark them as passing with TEST_PASSES_SANITIZE_LEAK=true. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Sep 1, 2022 at 01:14 UTC e77b3da6bb60e9af5963c9e42442afe53af1780b

6 files changed +12 -5

builtin/submodule--helper.c

+5 -5

@@ -1587,13 +1587,12 @@ static int clone_submodule(const struct module_clone_data *clone_data,

1587 char *sm_gitdir = clone_submodule_sm_gitdir(clone_data->name);

1588 char *sm_alternate = NULL, *error_strategy = NULL;

1589 struct child_process cp = CHILD_PROCESS_INIT;

1590 - const char *clone_data_path;

1590 + const char *clone_data_path = clone_data->path;

1591 + char *to_free = NULL;

1592

1593 if (!is_absolute_path(clone_data->path))

1593 - clone_data_path = xstrfmt("%s/%s", get_git_work_tree(),

1594 - clone_data->path);

1595 - else

1596 - clone_data_path = xstrdup(clone_data->path);

1594 + clone_data_path = to_free = xstrfmt("%s/%s", get_git_work_tree(),

1595 + clone_data->path);

1596

1597 if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0)

1598 die(_("refusing to create/use '%s' in another submodule's "

@@ -1678,6 +1677,7 @@ static int clone_submodule(const struct module_clone_data *clone_data,

1677

1678 free(sm_gitdir);

1679 free(p);

1680 + free(to_free);

1681 return 0;

1682 }

1683

t/t1500-rev-parse.sh

@@ -4,6 +4,7 @@ test_description='test git rev-parse'

4 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main

5 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME

7 +TEST_PASSES_SANITIZE_LEAK=true

8 . ./test-lib.sh

10 test_one () {

t/t6008-rev-list-submodule.sh

@@ -8,6 +8,7 @@ test_description='git rev-list involving submodules that this repo has'

8 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main

9 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME

11 +TEST_PASSES_SANITIZE_LEAK=true

12 . ./test-lib.sh

14 test_expect_success 'setup' '

t/t7414-submodule-mistakes.sh

@@ -1,6 +1,8 @@

1 #!/bin/sh

3 test_description='handling of common mistakes people may make with submodules'

4 +

5 +TEST_PASSES_SANITIZE_LEAK=true

6 . ./test-lib.sh

8 test_expect_success 'create embedded repository' '

t/t7506-status-submodule.sh

@@ -2,6 +2,7 @@

3 test_description='git status for submodule'

5 +TEST_PASSES_SANITIZE_LEAK=true

6 . ./test-lib.sh

8 test_create_repo_with_commit () {

t/t7507-commit-verbose.sh

@@ -1,6 +1,8 @@

1 #!/bin/sh

3 test_description='verbose commit template'

4 +

5 +TEST_PASSES_SANITIZE_LEAK=true

6 . ./test-lib.sh

8 write_script "check-for-diff" <<\EOF &&