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>

Patrick Steinhardt committed Aug 14, 2024 at 08:52 UTC 38678e5df55b77e98b47e7847faf83451eb161de
7 files changed +43 -11
range-diff.c
+4 -2
@@ -450,8 +450,10 @@ static void output_pair_header(struct diff_options *diffopt,
450 }
451
452 static struct userdiff_driver section_headers = {
453 - .funcname = { "^ ## (.*) ##$\n"
454 - "^.?@@ (.*)$", REG_EXTENDED }
453 + .funcname = {
454 + .pattern = "^ ## (.*) ##$\n^.?@@ (.*)$",
455 + .cflags = REG_EXTENDED,
456 + },
457 };
458
459 static struct diff_filespec *get_filespec(const char *name, const char *p)
t/t4018-diff-funcname.sh
+1
@@ -5,6 +5,7 @@
5
6 test_description='Test custom diff function name patterns'
7
8 +TEST_PASSES_SANITIZE_LEAK=true
9 . ./test-lib.sh
10
11 test_expect_success 'setup' '
t/t4042-diff-textconv-caching.sh
+2
@@ -1,6 +1,8 @@
1 #!/bin/sh
2
3 test_description='test textconv caching'
4 +
5 +TEST_PASSES_SANITIZE_LEAK=true
6 . ./test-lib.sh
7
8 cat >helper <<'EOF'
t/t4048-diff-combined-binary.sh
+1
@@ -4,6 +4,7 @@ test_description='combined and merge diff handle binary files and textconv'
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 binary merge conflict' '
t/t4209-log-pickaxe.sh
+2
@@ -1,6 +1,8 @@
1 #!/bin/sh
2
3 test_description='log --grep/--author/--regexp-ignore-case/-S/-G'
4 +
5 +TEST_PASSES_SANITIZE_LEAK=true
6 . ./test-lib.sh
7
8 test_log () {
userdiff.c
+29 -9
@@ -399,8 +399,11 @@ static struct userdiff_driver *userdiff_find_by_namelen(const char *name, size_t
399 static int parse_funcname(struct userdiff_funcname *f, const char *k,
400 const char *v, int cflags)
401 {
402 - if (git_config_string((char **) &f->pattern, k, v) < 0)
402 + f->pattern = NULL;
403 + FREE_AND_NULL(f->pattern_owned);
404 + if (git_config_string(&f->pattern_owned, k, v) < 0)
405 return -1;
406 + f->pattern = f->pattern_owned;
407 f->cflags = cflags;
408 return 0;
409 }
@@ -444,20 +447,37 @@ int userdiff_config(const char *k, const char *v)
447 return parse_funcname(&drv->funcname, k, v, REG_EXTENDED);
448 if (!strcmp(type, "binary"))
449 return parse_tristate(&drv->binary, k, v);
447 - if (!strcmp(type, "command"))
448 - return git_config_string((char **) &drv->external.cmd, k, v);
450 + if (!strcmp(type, "command")) {
451 + FREE_AND_NULL(drv->external.cmd);
452 + return git_config_string(&drv->external.cmd, k, v);
453 + }
454 if (!strcmp(type, "trustexitcode")) {
455 drv->external.trust_exit_code = git_config_bool(k, v);
456 return 0;
457 }
453 - if (!strcmp(type, "textconv"))
454 - return git_config_string((char **) &drv->textconv, k, v);
458 + if (!strcmp(type, "textconv")) {
459 + int ret;
460 + FREE_AND_NULL(drv->textconv_owned);
461 + ret = git_config_string(&drv->textconv_owned, k, v);
462 + drv->textconv = drv->textconv_owned;
463 + return ret;
464 + }
465 if (!strcmp(type, "cachetextconv"))
466 return parse_bool(&drv->textconv_want_cache, k, v);
457 - if (!strcmp(type, "wordregex"))
458 - return git_config_string((char **) &drv->word_regex, k, v);
459 - if (!strcmp(type, "algorithm"))
460 - return git_config_string((char **) &drv->algorithm, k, v);
467 + if (!strcmp(type, "wordregex")) {
468 + int ret;
469 + FREE_AND_NULL(drv->word_regex_owned);
470 + ret = git_config_string(&drv->word_regex_owned, k, v);
471 + drv->word_regex = drv->word_regex_owned;
472 + return ret;
473 + }
474 + if (!strcmp(type, "algorithm")) {
475 + int ret;
476 + FREE_AND_NULL(drv->algorithm_owned);
477 + ret = git_config_string(&drv->algorithm_owned, k, v);
478 + drv->algorithm = drv->algorithm_owned;
479 + return ret;
480 + }
481
482 return 0;
483 }
userdiff.h
+4
@@ -8,6 +8,7 @@ struct repository;
8
9 struct userdiff_funcname {
10 const char *pattern;
11 + char *pattern_owned;
12 int cflags;
13 };
14
@@ -20,11 +21,14 @@ struct userdiff_driver {
21 const char *name;
22 struct external_diff external;
23 const char *algorithm;
24 + char *algorithm_owned;
25 int binary;
26 struct userdiff_funcname funcname;
27 const char *word_regex;
28 + char *word_regex_owned;
29 const char *word_regex_multi_byte;
30 const char *textconv;
31 + char *textconv_owned;
32 struct notes_cache *textconv_cache;
33 int textconv_want_cache;
34 };