From 244c27242f44e6b88e3a381c90bde08d134c274b Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Wed, 16 Feb 2022 11:56:28 +0100 Subject: diff.[ch]: have diff_free() call clear_pathspec(opts.pathspec) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Have the diff_free() function call clear_pathspec(). Since the diff_flush() function calls this all its callers can be simplified to rely on it instead. When I added the diff_free() function in e900d494dcf (diff: add an API for deferred freeing, 2021-02-11) I simply missed this, or wasn't interested in it. Let's consolidate this now. This means that any future callers (and I've got revision.c in mind) that embed a "struct diff_options" can simply call diff_free() instead of needing know that it has an embedded pathspec. This does fix a bunch of leaks, but I can't mark any test here as passing under the SANITIZE=leak testing mode because in 886e1084d78 (builtin/: add UNLEAKs, 2017-10-01) an UNLEAK(rev) was added, which plasters over the memory leak. E.g. "t4011-diff-symlink.sh" would report fewer leaks with this fix, but because of the UNLEAK() reports none. I'll eventually loop around to removing that UNLEAK(rev) annotation as I'll fix deeper issues with the revisions API leaking. This is one small step on the way there, a new freeing function in revisions.c will want to call this diff_free(). Signed-off-by: Ævar Arnfjörð Bjarmason Reviewed-by: Elijah Newren Signed-off-by: Junio C Hamano --- diff.c | 1 + 1 file changed, 1 insertion(+) (limited to 'diff.c') diff --git a/diff.c b/diff.c index c862771a58..0aef3db6e1 100644 --- a/diff.c +++ b/diff.c @@ -6345,6 +6345,7 @@ void diff_free(struct diff_options *options) diff_free_file(options); diff_free_ignore_regex(options); + clear_pathspec(&options->pathspec); } void diff_flush(struct diff_options *options) -- cgit v1.2.3 From 6ee36364eb32287f071878a91d3bbcd86313754a Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Wed, 16 Feb 2022 11:56:29 +0100 Subject: diff.[ch]: have diff_free() free options->parseopts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "struct option" added in 4a288478394 (diff.c: prepare to use parse_options() for parsing, 2019-01-27) would be free'd in the case of diff_setup_done() being called. But not all codepaths that allocate it reach that, e.g. "t6427-diff3-conflict-markers.sh" will now free memory that it didn't free before. By using FREE_AND_NULL() here (which diff_setup_done() also does) we ensure that we free the memory, and that we won't have double-free's. Before this running: ./t6427-diff3-conflict-markers.sh -vixd --run=7 Would report: SUMMARY: LeakSanitizer: 7823 byte(s) leaked in 6 allocation(s). But now we'll report: SUMMARY: LeakSanitizer: 703 byte(s) leaked in 5 allocation(s). I.e. the largest leak in that particular test has now been addressed. Signed-off-by: Ævar Arnfjörð Bjarmason Reviewed-by: Elijah Newren Signed-off-by: Junio C Hamano --- diff.c | 1 + 1 file changed, 1 insertion(+) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 0aef3db6e1..fb8bc8aadb 100644 --- a/diff.c +++ b/diff.c @@ -6346,6 +6346,7 @@ void diff_free(struct diff_options *options) diff_free_file(options); diff_free_ignore_regex(options); clear_pathspec(&options->pathspec); + FREE_AND_NULL(options->parseopts); } void diff_flush(struct diff_options *options) -- cgit v1.2.3