Re: [PATCH] describe: fix --no-exact-match

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 9, 2023, 19:07 UTC
Message-ID: <xmqqzg30m3vo.fsf@gitster.g>
In-Reply-To: <22e5a87a-cd35-9793-5b6f-6eb368fdf40e@web.de>

René Scharfe <l.s.r@web.de> writes:

Show 23 quoted lines

>> Here's what it looks like, for reference.
>>
>> diff --git a/builtin/describe.c b/builtin/describe.c
>> index b28a4a1f82..718b5c3073 100644
>> --- a/builtin/describe.c
>> +++ b/builtin/describe.c
>> @@ -561,9 +561,11 @@ static void describe(const char *arg, int last_one)
>>  static int option_parse_exact_match(const struct option *opt, const char *arg,
>>  				    int unset)
>>  {
>> +	int *val = opt->value;
>
> This line would assign opt->value_int instead...
>
>> +
>>  	BUG_ON_OPT_ARG(arg);
>>
>> -	max_candidates = unset ? DEFAULT_CANDIDATES : 0;
>> +	*val = unset ? DEFAULT_CANDIDATES : 0;
>>  	return 0;
>>  }
>> ...
> Thoughts?  Too much churn?

Sorry, but I am not sure what is being proposed. opt->value_int would presumably be defined as a pointer to an int. The calling side presumably defines the options[] array entry so that the &max_candidates address is given to opt->value_int and not to opt->value_someothertype, and if you try to pass a pointer of a wrong type, it would be caught as a type mismatch by the compiler, so the above covers both ends (i.e. the address &max_candidates is passed in the correct member of the struct, and the callback takes val out of the correct member of the struct), as long as the callback uses the right type. If &max_candidates is int now, and the above code is written to use opt->value_int, anybody who needs to update max_candidates to ulong must make sure three things match, i.e. the type of max_candidates variable itself, the member the caller uses to pass &max_candidates in the struct (i.e. change to use value_ulong and not value_int), and the type of the local variable the callback function uses.

But I am failing to imagine how the calling side actually would look like. Can we do something along the lines of

    #define OPT_CALLBACK(s, l, v, a, h, cb)
	.short_name = (s),
	.long_name = (l),
	.value_ ## typeof(v) = &v,
	.help = (h),
	.callback = (cb),

with a clever CPP trick? It sounds like either too much churn or too much magic or both, at least to me.

Thanks.