userdiff: fix leaking memory for configured diff drivers

The userdiff structures may be initialized either statically on the
stack or dynamically via configuration keys. In the latter case we end
up leaking memory because we didn't have any infrastructure to discern
those strings which have been allocated statically and those which have
been allocated dynamically.

Refactor the code such that we have two pointers for each of these
strings: one that holds the value as accessed by other subsystems, and
one that points to the same string in case it has been allocated. Like
this, we can safely free the second pointer and thus plug those memory
leaks.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/range-diff.c b/range-diff.c
index 5f01605550e57206accbf166a782681db4087eef..bbb0952264b817efc6d57c2348ec5f980d2a30ec 100644 (file)
--- a/range-diff.c
+++ b/range-diff.c
@@ -450,8 +450,10 @@ static void output_pair_header(struct diff_options *diffopt,
 }
 
 static struct userdiff_driver section_headers = {
-       .funcname = { "^ ## (.*) ##$\n"
-                     "^.?@@ (.*)$", REG_EXTENDED }
+       .funcname = {
+               .pattern = "^ ## (.*) ##$\n^.?@@ (.*)$",
+               .cflags = REG_EXTENDED,
+       },
 };
 
 static struct diff_filespec *get_filespec(const char *name, const char *p)

diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index e026fac1f4090330f81b5b417e1b9e930795fd96..8128c30e7f2c6517a5ca0cf03cd22c1693978a5d 100755 (executable)
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -5,6 +5,7 @@
 
 test_description='Test custom diff function name patterns'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'setup' '

diff --git a/t/t4042-diff-textconv-caching.sh b/t/t4042-diff-textconv-caching.sh
index 8ebfa3c1be2fec46558c739fd3ccda37a952c225..a179205394d8a33b4ab7be0d7e700739448a5597 100755 (executable)
--- a/t/t4042-diff-textconv-caching.sh
+++ b/t/t4042-diff-textconv-caching.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='test textconv caching'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 cat >helper <<'EOF'

diff --git a/t/t4048-diff-combined-binary.sh b/t/t4048-diff-combined-binary.sh
index 0260cf64f5d5507eb53fde74fba88d9d3a9ab3b4..f399484bcef623204d44e3915b99f2de48ff95ae 100755 (executable)
--- a/t/t4048-diff-combined-binary.sh
+++ b/t/t4048-diff-combined-binary.sh
@@ -4,6 +4,7 @@ test_description='combined and merge diff handle binary files and textconv'
 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 binary merge conflict' '

diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
index 64e16237335dbb37e3d250f248758017df62943b..b42fdc54fcb544349dd2fdb7fd8f48b0bf241442 100755 (executable)
--- a/t/t4209-log-pickaxe.sh
+++ b/t/t4209-log-pickaxe.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='log --grep/--author/--regexp-ignore-case/-S/-G'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_log () {

diff --git a/userdiff.c b/userdiff.c
index c4ebb9ff73464ad20a5e78f93625a192e45ecd93..989629149f668e68f116a109b5fd449de5d4b003 100644 (file)
--- a/userdiff.c
+++ b/userdiff.c
@@ -399,8 +399,11 @@ static struct userdiff_driver *userdiff_find_by_namelen(const char *name, size_t
 static int parse_funcname(struct userdiff_funcname *f, const char *k,
                const char *v, int cflags)
 {
-       if (git_config_string((char **) &f->pattern, k, v) < 0)
+       f->pattern = NULL;
+       FREE_AND_NULL(f->pattern_owned);
+       if (git_config_string(&f->pattern_owned, k, v) < 0)
                return -1;
+       f->pattern = f->pattern_owned;
        f->cflags = cflags;
        return 0;
 }
@@ -444,20 +447,37 @@ int userdiff_config(const char *k, const char *v)
                return parse_funcname(&drv->funcname, k, v, REG_EXTENDED);
        if (!strcmp(type, "binary"))
                return parse_tristate(&drv->binary, k, v);
-       if (!strcmp(type, "command"))
-               return git_config_string((char **) &drv->external.cmd, k, v);
+       if (!strcmp(type, "command")) {
+               FREE_AND_NULL(drv->external.cmd);
+               return git_config_string(&drv->external.cmd, k, v);
+       }
        if (!strcmp(type, "trustexitcode")) {
                drv->external.trust_exit_code = git_config_bool(k, v);
                return 0;
        }
-       if (!strcmp(type, "textconv"))
-               return git_config_string((char **) &drv->textconv, k, v);
+       if (!strcmp(type, "textconv")) {
+               int ret;
+               FREE_AND_NULL(drv->textconv_owned);
+               ret = git_config_string(&drv->textconv_owned, k, v);
+               drv->textconv = drv->textconv_owned;
+               return ret;
+       }
        if (!strcmp(type, "cachetextconv"))
                return parse_bool(&drv->textconv_want_cache, k, v);
-       if (!strcmp(type, "wordregex"))
-               return git_config_string((char **) &drv->word_regex, k, v);
-       if (!strcmp(type, "algorithm"))
-               return git_config_string((char **) &drv->algorithm, k, v);
+       if (!strcmp(type, "wordregex")) {
+               int ret;
+               FREE_AND_NULL(drv->word_regex_owned);
+               ret = git_config_string(&drv->word_regex_owned, k, v);
+               drv->word_regex = drv->word_regex_owned;
+               return ret;
+       }
+       if (!strcmp(type, "algorithm")) {
+               int ret;
+               FREE_AND_NULL(drv->algorithm_owned);
+               ret = git_config_string(&drv->algorithm_owned, k, v);
+               drv->algorithm = drv->algorithm_owned;
+               return ret;
+       }
 
        return 0;
 }

diff --git a/userdiff.h b/userdiff.h
index 7565930337cf5546e8b316d3725e630917f8f7d1..827361b0bc9569426ba35d5b47b066464d1b7b92 100644 (file)
--- a/userdiff.h
+++ b/userdiff.h
@@ -8,6 +8,7 @@ struct repository;
 
 struct userdiff_funcname {
        const char *pattern;
+       char *pattern_owned;
        int cflags;
 };
 
@@ -20,11 +21,14 @@ struct userdiff_driver {
        const char *name;
        struct external_diff external;
        const char *algorithm;
+       char *algorithm_owned;
        int binary;
        struct userdiff_funcname funcname;
        const char *word_regex;
+       char *word_regex_owned;
        const char *word_regex_multi_byte;
        const char *textconv;
+       char *textconv_owned;
        struct notes_cache *textconv_cache;
        int textconv_want_cache;
 };