git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 3/3] diff: remove parseopts member from struct diff_options

From
René Scharfe <l.s.r@web.de>
Date
Dec 1, 2022, 13:44 UTC
Message-ID
<ec70aee7-2cca-c184-930d-e61a6f4597c3@web.de>
In-Reply-To
<ea838ae7-b635-d4d2-d9df-e96b3d8980af@web.de>

repo_diff_setup() builds the struct option array with git diff's command line options and stores a pointer to it in the parseopts member of struct diff_options. The array is freed by diff_setup_done(), but not by release_revisions(). Thus calling only repo_diff_setup() and release_revisions() leaks that array.

We could free it in release_revisions() as well to plug that leak, but there is a better way: Only build it when needed. Move the get_diff_parseopts() calls to the two places that use the array, free it after use and get rid of the parseopts member.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 diff.c | 17 ++++++++---------
 diff.h |  1 -
 2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/diff.c b/diff.c
index e469d5d2a0..6415c4dc2d 100644
--- a/diff.c
+++ b/diff.c
@@ -4615,8 +4615,6 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
 	builtin_checkdiff(name, other, attr_path, p->one, p->two, o);
 }

-static struct option *get_diff_parseopts(struct diff_options *options);
-
 void repo_diff_setup(struct repository *r, struct diff_options *options)
 {
 	memcpy(options, &default_diff_options, sizeof(*options));
@@ -4662,8 +4660,6 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)

 	options->color_moved = diff_color_moved_default;
 	options->color_moved_ws_handling = diff_color_moved_ws_default;
-
-	options->parseopts = get_diff_parseopts(options);
 }

 static const char diff_status_letters[] = {
@@ -4821,8 +4817,6 @@ void diff_setup_done(struct diff_options *options)
 			options->filter = ~filter_bit[DIFF_STATUS_FILTER_AON];
 		options->filter &= ~options->filter_not;
 	}
-
-	FREE_AND_NULL(options->parseopts);
 }

 int parse_long_opt(const char *opt, const char **argv,
@@ -5695,21 +5689,27 @@ static struct option *get_diff_parseopts(struct diff_options *options)
 struct option *add_diff_options(const struct option *parseopts,
 				struct diff_options *options)
 {
-	return parse_options_concat(parseopts, options->parseopts);
+	struct option *diff_parseopts = get_diff_parseopts(options);
+	struct option *result = parse_options_concat(parseopts, diff_parseopts);
+	free(diff_parseopts);
+	return result;
 }

 int diff_opt_parse(struct diff_options *options,
 		   const char **av, int ac, const char *prefix)
 {
+	struct option *diff_parseopts = get_diff_parseopts(options);
+
 	if (!prefix)
 		prefix = "";

-	ac = parse_options(ac, av, prefix, options->parseopts, NULL,
+	ac = parse_options(ac, av, prefix, diff_parseopts, NULL,
 			   PARSE_OPT_KEEP_DASHDASH |
 			   PARSE_OPT_KEEP_UNKNOWN_OPT |
 			   PARSE_OPT_NO_INTERNAL_HELP |
 			   PARSE_OPT_ONE_SHOT |
 			   PARSE_OPT_STOP_AT_NON_OPTION);
+	free(diff_parseopts);

 	return ac;
 }
@@ -6518,7 +6518,6 @@ 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)
diff --git a/diff.h b/diff.h
index c20a1ad76d..41eb2c3d42 100644
--- a/diff.h
+++ b/diff.h
@@ -394,7 +394,6 @@ struct diff_options {
 	unsigned color_moved_ws_handling;

 	struct repository *repo;
-	struct option *parseopts;
 	struct strmap *additional_path_headers;

 	int no_free;
--
2.38.1
Previous: René ScharfeNext: Ævar Arnfjörð Bjarmason
Message 14 of 23 in “diff: build parseopts array on demand”
  1. 0/3 diff: build parseopts array on demandRené Scharfe, Nov 30, 2022
  2. 1/3 diff: factor out add_diff_options()René Scharfe, Nov 30, 2022
  3. ZheNing HuDec 1, 2022
  4. 2/3 diff: let prep_parse_options() return parseopt arrayRené Scharfe, Nov 30, 2022
  5. 3/3 diff: remove parseopts member of struct diff_optionsRené Scharfe, Nov 30, 2022
  6. Junio C HamanoDec 1, 2022
  7. René ScharfeDec 1, 2022
  8. Junio C HamanoDec 1, 2022
  9. René ScharfeDec 1, 2022
  10. Junio C HamanoDec 1, 2022
  11. 0/3 diff: build parseopts array on demandRené Scharfe, Dec 1, 2022
  12. 1/3 diff: factor out add_diff_options()René Scharfe, Dec 1, 2022
  13. 2/3 diff: let prep_parse_options() return parseopt arrayRené Scharfe, Dec 1, 2022
  14. 3/3 diff: remove parseopts member from struct diff_optionsRené Scharfe, Dec 1, 2022
  15. Ævar Arnfjörð BjarmasonDec 1, 2022
  16. René ScharfeDec 1, 2022
  17. Eric SunshineDec 1, 2022
  18. René ScharfeDec 1, 2022
  19. Ævar Arnfjörð BjarmasonDec 1, 2022
  20. 0/3 diff: build parseopts array on demandRené Scharfe, Dec 1, 2022
  21. 1/3 diff: factor out add_diff_options()René Scharfe, Dec 1, 2022
  22. 2/3 diff: use add_diff_options() in diff_opt_parse()René Scharfe, Dec 1, 2022
  23. 3/3 diff: remove parseopts member from struct diff_optionsRené Scharfe, Dec 1, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.