Skip to content

Commit fe4c750

Browse files
avargitster
authored andcommitted
submodule--helper: fix a configure_added_submodule() leak
Fix config API a memory leak added in a452128 (submodule--helper: introduce add-config subcommand, 2021-08-06) by using the *_tmp() variant of git_config_get_string(). In this case we're only checking whether the (repo|git)_config_get_string() call is telling us that the "submodule.active" key exists. As with the preceding commit we'll find many other such patterns in the codebase if we go fishing. E.g. "git gc" leaks in the code added in 61f7a38 (maintenance: use 'incremental' strategy by default, 2020-10-15). Similar code in "git gc" added in b08ff1f (maintenance: add --schedule option and config, 2020-09-11) doesn't leak, but we could avoid the malloc() & free() in that case. A coccinelle rule to find those would find and fix some leaks, and cases where we're doing needless malloc() + free()'s but only care about the key existence, or are copying the (repo|git)_config_get_string() return value right away. But as with the preceding commit let's punt on all of that for now, and just narrowly fix this specific case in submodule--helper. Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Reviewed-by: Glen Choo <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 4c4d3e7 commit fe4c750

File tree

2 files changed

+3
-2
lines changed

2 files changed

+3
-2
lines changed

builtin/submodule--helper.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3157,7 +3157,7 @@ static int config_submodule_in_gitmodules(const char *name, const char *var, con
31573157
static void configure_added_submodule(struct add_data *add_data)
31583158
{
31593159
char *key;
3160-
char *val = NULL;
3160+
const char *val;
31613161
struct child_process add_submod = CHILD_PROCESS_INIT;
31623162
struct child_process add_gitmodules = CHILD_PROCESS_INIT;
31633163

@@ -3202,7 +3202,7 @@ static void configure_added_submodule(struct add_data *add_data)
32023202
* is_submodule_active(), since that function needs to find
32033203
* out the value of "submodule.active" again anyway.
32043204
*/
3205-
if (!git_config_get_string("submodule.active", &val)) {
3205+
if (!git_config_get_string_tmp("submodule.active", &val)) {
32063206
/*
32073207
* If the submodule being added isn't already covered by the
32083208
* current configured pathspec, set the submodule's active flag

t/t7413-submodule-is-active.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ This is a unit test of the submodule.c is_submodule_active() function,
99
which is also indirectly tested elsewhere.
1010
'
1111

12+
TEST_PASSES_SANITIZE_LEAK=true
1213
. ./test-lib.sh
1314

1415
test_expect_success 'setup' '

0 commit comments

Comments
 (0)