Re: [PATCH v8 0/2] builtin/config.c: support `--type=<type>` as preferred alias for `--type`

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

 



Taylor Blau <me@xxxxxxxxxxxx> writes:

>> > +#define OPT_CALLBACK_VALUE(s, l, h, f, i) \
>> > +	{ OPTION_CALLBACK, (s), (l), NULL, NULL, (h), PARSE_OPT_NOARG | \
>> > +	PARSE_OPT_NONEG, (f), (i) }
>> > +
>> > +static struct option builtin_config_options[];
>>
>> OK.  I am not sure if OPT_CALLBACK_VALUE() needs to take 'f', as you
>> always pass the option_parse_type function to it.
>
> That's fair. I left this in as an indication that something like this
> _might_ want to make its way into parse-options.h as a general-purpose
> utility, but was not yet ready to do so. Thus, I defined it inside
> builtin/config.c.

I understood the reasoning, but as your current verdict is that this
is not yet ready for parse-options.[ch], I think it is probably
preferrable to reduce repeated passing of the same function to the
macro, at least while it is in this builgin/config.c file.

> @@ -71,7 +71,7 @@ static int option_parse_type(const struct option *opt, const char *arg,
>  			     int unset)
>  {
>  	if (unset) {
> -		type = 0;
> +		*((int *) opt->value) = 0;
>  		return 0;
>  	}

Yup.  This (if it works) is exactly what I imagined the function
should look like.



[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]

  Powered by Linux