Hi Junio, On Mon, 10 Oct 2016, Junio C Hamano wrote: > Johannes Schindelin <johannes.schindelin@xxxxxx> writes: > > > 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