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 5f01605..bbb0952 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 e026fac..8128c30 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 8ebfa3c..a179205 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 0260cf6..f399484 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 64e1623..b42fdc5 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 c4ebb9f..9896291 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 7565930..827361b 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;
 };