From c5630c4868c74aab2fc4e8a4ef4bfc9c51534e0e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= <l.s.r@web.de>
Date: Thu, 1 Dec 2022 23:49:11 +0100
Subject: [PATCH 1/3] diff: factor out add_diff_options()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add a function for appending the parseopts member of struct diff_options
to a struct option array. Use it in two sites instead of accessing the
parseopts member directly. Decoupling callers from diff internals like
that allows us to change the latter.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/range-diff.c | 2 +-
diff-no-index.c | 3 +--
diff.c | 6 ++++++
diff.h | 1 +
4 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index e2a74efb42..aecfae12d3 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -47,7 +47,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
repo_diff_setup(the_repository, &diffopt);
- options = parse_options_concat(range_diff_options, diffopt.parseopts);
+ options = add_diff_options(range_diff_options, &diffopt);
argc = parse_options(argc, argv, prefix, options,
builtin_range_diff_usage, PARSE_OPT_KEEP_DASHDASH);
diff --git a/diff-no-index.c b/diff-no-index.c
index 18edbdf4b5..05fafd0019 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -255,8 +255,7 @@ int diff_no_index(struct rev_info *revs,
};
struct option *options;
- options = parse_options_concat(no_index_options,
- revs->diffopt.parseopts);
+ options = add_diff_options(no_index_options, &revs->diffopt);
argc = parse_options(argc, argv, revs->prefix, options,
diff_no_index_usage, 0);
if (argc != 2) {
diff --git a/diff.c b/diff.c
index 648f6717a5..a74bfb4a60 100644
--- a/diff.c
+++ b/diff.c
@@ -5397,6 +5397,12 @@ static int diff_opt_rotate_to(const struct option *opt, const char *arg, int uns
return 0;
}
+struct option *add_diff_options(const struct option *opts,
+ struct diff_options *options)
+{
+ return parse_options_concat(opts, options->parseopts);
+}
+
static void prep_parse_options(struct diff_options *options)
{
struct option parseopts[] = {
diff --git a/diff.h b/diff.h
index 8ae18e5ab1..5229f20486 100644
--- a/diff.h
+++ b/diff.h
@@ -539,6 +539,7 @@ int git_diff_ui_config(const char *var, const char *value, void *cb);
#define diff_setup(diffopts) repo_diff_setup(the_repository, diffopts)
#endif
void repo_diff_setup(struct repository *, struct diff_options *);
+struct option *add_diff_options(const struct option *, struct diff_options *);
int diff_opt_parse(struct diff_options *, const char **, int, const char *);
void diff_setup_done(struct diff_options *);
int git_config_rename(const char *var, const char *value);
From 6c6048fa7f1cf7b9ae79beb0f57fd03dd4021bb4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= <l.s.r@web.de>
Date: Thu, 1 Dec 2022 23:51:21 +0100
Subject: [PATCH 2/3] diff: use add_diff_options() in diff_opt_parse()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Prepare the removal of the parseopts member of struct diff_options by
using the API function add_diff_options() instead of accessing it
directly to get the command line option definitions. Building the copy
by concatenating with an empty option array is slightly awkward, but
simpler than a non-concat version of add_diff_options() would be to use
in places that need concatenation.
Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index a74bfb4a60..832af35c91 100644
--- a/diff.c
+++ b/diff.c
@@ -5680,15 +5680,19 @@ static void prep_parse_options(struct diff_options *options)
int diff_opt_parse(struct diff_options *options,
const char **av, int ac, const char *prefix)
{
+ struct option no_options[] = { OPT_END() };
+ struct option *parseopts = add_diff_options(no_options, options);
+
if (!prefix)
prefix = "";
- ac = parse_options(ac, av, prefix, options->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(parseopts);
return ac;
}
From 189e97bc4ba8d5f2ab4746d41204feed159fbe9a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= <l.s.r@web.de>
Date: Thu, 1 Dec 2022 23:53:17 +0100
Subject: [PATCH 3/3] diff: remove parseopts member from struct diff_options
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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. Absorb
prep_parse_options() into the last place that uses the parseopts member
of struct diff_options, add_diff_parseopts(), and get rid of said
member.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 15 +--------------
diff.h | 1 -
2 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/diff.c b/diff.c
index 832af35c91..6741a16d02 100644
--- a/diff.c
+++ b/diff.c
@@ -4593,8 +4593,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 void prep_parse_options(struct diff_options *options);
-
void repo_diff_setup(struct repository *r, struct diff_options *options)
{
memcpy(options, &default_diff_options, sizeof(*options));
@@ -4640,8 +4638,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;
-
- prep_parse_options(options);
}
static const char diff_status_letters[] = {
@@ -4799,8 +4795,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,
@@ -5399,11 +5393,6 @@ static int diff_opt_rotate_to(const struct option *opt, const char *arg, int uns
struct option *add_diff_options(const struct option *opts,
struct diff_options *options)
-{
- return parse_options_concat(opts, options->parseopts);
-}
-
-static void prep_parse_options(struct diff_options *options)
{
struct option parseopts[] = {
OPT_GROUP(N_("Diff output format options")),
@@ -5673,8 +5662,7 @@ static void prep_parse_options(struct diff_options *options)
OPT_END()
};
- ALLOC_ARRAY(options->parseopts, ARRAY_SIZE(parseopts));
- memcpy(options->parseopts, parseopts, sizeof(parseopts));
+ return parse_options_concat(opts, parseopts);
}
int diff_opt_parse(struct diff_options *options,
@@ -6497,7 +6485,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 5229f20486..6840499844 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;