Re: [PATCH v3 05/25] sequencer: eventually release memory allocated for the option values
From: Johannes Schindelin <hidden>
Date: 2016-10-11 10:08:09
Hi Junio, On Mon, 10 Oct 2016, Junio C Hamano wrote:
quoted hunk ↗ jump to hunk
Johannes Schindelin [off-list ref] writes:quoted
diff --git a/builtin/revert.c b/builtin/revert.c index 7365559..fce9c75 100644 --- a/builtin/revert.c +++ b/builtin/revert.c@@ -174,6 +174,12 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) if (argc > 1) usage_with_options(usage_str, options); + + /* These option values will be free()d */ + if (opts->gpg_sign) + opts->gpg_sign = xstrdup(opts->gpg_sign); + if (opts->strategy) + opts->strategy = xstrdup(opts->strategy); }This certainly is good, but I wonder if a new variant of OPT_STRING and OPTION_STRING that does the strdup for you, something along the lines of ...diff --git a/parse-options.c b/parse-options.c index 312a85dbde..6aab6b0b05 100644 --- a/parse-options.c +++ b/parse-options.c@@ -138,6 +138,21 @@ static int get_value(struct parse_opt_ctx_t *p, return get_arg(p, opt, flags, (const char **)opt->value); return 0; + case OPTION_STRDUP: + err = 0; + free(opt->value);
Probably free(*(char **)opt->value); instead.
+ if (unset)
+ *(const char **)opt->value = NULL;
+ else if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
+ *(const char **)opt->value = xstrdup(opt->defval);
+ else {
+ const char *v;
+ err = get_arg(p, opt, flags, &v);
+ if (!err)
+ *(const char **)opt->value = xstrdup(v);
+ }
+ return err;
+
case OPTION_FILENAME:
err = 0;
if (unset)
... may make it even more pleasant to use?With s/even// I would agree. I will keep this patch in mind and will try to come back to it, once the rebase--helper patches are well on target for `master`. Ciao, Dscho