@matheusbsbs / matheus-bagliano / commits / 244c27242f

diff.[ch]: have diff_free() call clear_pathspec(opts.pathspec)

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 <avarab@gmail.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Feb 16, 2022 at 11:56 UTC 244c27242f44e6b88e3a381c90bde08d134c274b

5 files changed +4 -9

add-interactive.c

+3 -3

@@ -797,14 +797,14 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps,

797 diffopt.flags.override_submodule_config = 1;

798 diffopt.repo = s->r;

799

800 - if (do_diff_cache(&oid, &diffopt))

800 + if (do_diff_cache(&oid, &diffopt)) {

801 + diff_free(&diffopt);

802 res = -1;

802 - else {

803 + } else {

804 diffcore_std(&diffopt);

805 diff_flush(&diffopt);

806 }

807 free(paths);

807 - clear_pathspec(&diffopt.pathspec);

808

809 if (!res && write_locked_index(s->r->index, &index_lock,

810 COMMIT_LOCK) < 0)

blame.c

-3

@@ -1403,7 +1403,6 @@ static struct blame_origin *find_origin(struct repository *r,

1403 }

1404 }

1405 diff_flush(&diff_opts);

1406 - clear_pathspec(&diff_opts.pathspec);

1406 return porigin;

1407 }

1408

@@ -1447,7 +1446,6 @@ static struct blame_origin *find_rename(struct repository *r,

1446 }

1447 }

1448 diff_flush(&diff_opts);

1450 - clear_pathspec(&diff_opts.pathspec);

1449 return porigin;

1450 }

1451

@@ -2328,7 +2326,6 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,

2326 } while (unblamed);

2327 target->suspects = reverse_blame(leftover, NULL);

2328 diff_flush(&diff_opts);

2331 - clear_pathspec(&diff_opts.pathspec);

2329 }

2330

2331 /*

builtin/reset.c

-1

@@ -274,7 +274,6 @@ static int read_from_tree(const struct pathspec *pathspec,

274 return 1;

275 diffcore_std(&opt);

276 diff_flush(&opt);

277 - clear_pathspec(&opt.pathspec);

277

278 return 0;

279 }

diff.c

@@ -6345,6 +6345,7 @@ void diff_free(struct diff_options *options)

6345

6346 diff_free_file(options);

6347 diff_free_ignore_regex(options);

6348 + clear_pathspec(&options->pathspec);

6349 }

6350

6351 void diff_flush(struct diff_options *options)

notes-merge.c

-2

@@ -175,7 +175,6 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,

175 oid_to_hex(&mp->remote));

176 }

177 diff_flush(&opt);

178 - clear_pathspec(&opt.pathspec);

178

179 *num_changes = len;

180 return changes;

@@ -261,7 +260,6 @@ static void diff_tree_local(struct notes_merge_options *o,

260 oid_to_hex(&mp->local));

261 }

262 diff_flush(&opt);

264 - clear_pathspec(&opt.pathspec);

263 }

264

265 static void check_notes_merge_worktree(struct notes_merge_options *o)