Re: [PATCH v3 05/25] sequencer: eventually release memory allocated for the option values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]