Skip to content

Commit 38678e5

Browse files
pks-tgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 1bc158e commit 38678e5

7 files changed

+43
-11
lines changed

range-diff.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,8 +450,10 @@ static void output_pair_header(struct diff_options *diffopt,
450450
}
451451

452452
static struct userdiff_driver section_headers = {
453-
.funcname = { "^ ## (.*) ##$\n"
454-
"^.?@@ (.*)$", REG_EXTENDED }
453+
.funcname = {
454+
.pattern = "^ ## (.*) ##$\n^.?@@ (.*)$",
455+
.cflags = REG_EXTENDED,
456+
},
455457
};
456458

457459
static struct diff_filespec *get_filespec(const char *name, const char *p)

t/t4018-diff-funcname.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
test_description='Test custom diff function name patterns'
77

8+
TEST_PASSES_SANITIZE_LEAK=true
89
. ./test-lib.sh
910

1011
test_expect_success 'setup' '

t/t4042-diff-textconv-caching.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#!/bin/sh
22

33
test_description='test textconv caching'
4+
5+
TEST_PASSES_SANITIZE_LEAK=true
46
. ./test-lib.sh
57

68
cat >helper <<'EOF'

t/t4048-diff-combined-binary.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ test_description='combined and merge diff handle binary files and textconv'
44
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
55
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
66

7+
TEST_PASSES_SANITIZE_LEAK=true
78
. ./test-lib.sh
89

910
test_expect_success 'setup binary merge conflict' '

t/t4209-log-pickaxe.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#!/bin/sh
22

33
test_description='log --grep/--author/--regexp-ignore-case/-S/-G'
4+
5+
TEST_PASSES_SANITIZE_LEAK=true
46
. ./test-lib.sh
57

68
test_log () {

userdiff.c

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -399,8 +399,11 @@ static struct userdiff_driver *userdiff_find_by_namelen(const char *name, size_t
399399
static int parse_funcname(struct userdiff_funcname *f, const char *k,
400400
const char *v, int cflags)
401401
{
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)
403405
return -1;
406+
f->pattern = f->pattern_owned;
404407
f->cflags = cflags;
405408
return 0;
406409
}
@@ -444,20 +447,37 @@ int userdiff_config(const char *k, const char *v)
444447
return parse_funcname(&drv->funcname, k, v, REG_EXTENDED);
445448
if (!strcmp(type, "binary"))
446449
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+
}
449454
if (!strcmp(type, "trustexitcode")) {
450455
drv->external.trust_exit_code = git_config_bool(k, v);
451456
return 0;
452457
}
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+
}
455465
if (!strcmp(type, "cachetextconv"))
456466
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+
}
461481

462482
return 0;
463483
}

userdiff.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ struct repository;
88

99
struct userdiff_funcname {
1010
const char *pattern;
11+
char *pattern_owned;
1112
int cflags;
1213
};
1314

@@ -20,11 +21,14 @@ struct userdiff_driver {
2021
const char *name;
2122
struct external_diff external;
2223
const char *algorithm;
24+
char *algorithm_owned;
2325
int binary;
2426
struct userdiff_funcname funcname;
2527
const char *word_regex;
28+
char *word_regex_owned;
2629
const char *word_regex_multi_byte;
2730
const char *textconv;
31+
char *textconv_owned;
2832
struct notes_cache *textconv_cache;
2933
int textconv_want_cache;
3034
};

0 commit comments

Comments
 (0)