[PATCH v3 0/3] diff: build parseopts array on demand

From: "René Scharfe" <l.s.r@web.de>
To: Git List <git@vger.kernel.org>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Jeff King" <peff@peff.net>,
	"ZheNing Hu" <adlternative@gmail.com>
Subject: [PATCH v3 0/3] diff: build parseopts array on demand
Date: Thu, 1 Dec 2022 23:45:55 +0100	[thread overview]
Message-ID: <19325420-5630-87bb-0c2c-362151214b36@web.de> (raw)
In-Reply-To: <0620b4a6-b80a-d8a0-5a35-857b9fe0c986@web.de>

Calling repo_init_revisions() and release_revisions() in that order
leaks the memory allocated for the parseopts array in the embedded
struct diff_options member.  Get rid of that leak by reducing the
lifetime of that array.

Original patch:
https://lore.kernel.org/git/4fd82dc6-e0f8-0638-5b10-16bfef39a171@web.de/

Submitted separately from that thread because it's independent enough.

Changes since v2:
- Inline get_diff_parseopts() to simplify the code.

Change since v1:
- Actually remove the parseopts member.  Its removal got lost during
  refactoring in v1.  Thank you for spotting that, Junio!


  diff: factor out add_diff_options()
  diff: use add_diff_options() in diff_opt_parse()
  diff: remove parseopts member from struct diff_options

 builtin/range-diff.c |  2 +-
 diff-no-index.c      |  3 +--
 diff.c               | 19 ++++++++-----------
 diff.h               |  2 +-
 4 files changed, 11 insertions(+), 15 deletions(-)

Interdiff against v2:

diff --git a/diff.c b/diff.c
index 6415c4dc2d..4dfe824c85 100644
--- a/diff.c
+++ b/diff.c
@@ -5413,7 +5413,8 @@ static int diff_opt_rotate_to(const struct option *opt, const char *arg, int uns
 	return 0;
 }

-static struct option *get_diff_parseopts(struct diff_options *options)
+struct option *add_diff_options(const struct option *opts,
+				struct diff_options *options)
 {
 	struct option parseopts[] = {
 		OPT_GROUP(N_("Diff output format options")),
@@ -5683,33 +5684,25 @@ static struct option *get_diff_parseopts(struct diff_options *options)
 		OPT_END()
 	};

-	return parse_options_dup(parseopts);
-}
-
-struct option *add_diff_options(const struct option *parseopts,
-				struct diff_options *options)
-{
-	struct option *diff_parseopts = get_diff_parseopts(options);
-	struct option *result = parse_options_concat(parseopts, diff_parseopts);
-	free(diff_parseopts);
-	return result;
+	return parse_options_concat(opts, parseopts);
 }

 int diff_opt_parse(struct diff_options *options,
 		   const char **av, int ac, const char *prefix)
 {
-	struct option *diff_parseopts = get_diff_parseopts(options);
+	struct option no_options[] = { OPT_END() };
+	struct option *parseopts = add_diff_options(no_options, options);

 	if (!prefix)
 		prefix = "";

-	ac = parse_options(ac, av, prefix, diff_parseopts, NULL,
+	ac = parse_options(ac, av, prefix, 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);
+	free(parseopts);

 	return ac;
 }
--
2.30.2
