Re: [PATCH 2/2] die_for_incompatible_opts(): accept more than four options

From: René Scharfe <l.s.r@web.de>
Date: Aug 29, 2026, 18:04 UTC
Message-ID: <5d5b1f26-192f-457d-bc18-499a3d7507fa@web.de>
In-Reply-To: <20260829111418.GA40814@coredump.intra.peff.net>

On 8/29/26 1:14 PM, Jeff King wrote:

Show 28 quoted lines

> On Thu, Aug 27, 2026 at 07:35:38AM -0700, Junio C Hamano wrote:
> 
>>> So that makes sense. Of course the follow-on question is whether any
>>> callers actually want to pass more than 4 options. I don't see any
>>> patches adding new calls.
>>
>> There isn't.  While I was writing [*], I wondered if the two calls
>> next to each other for opt3 and opt4 want to be combined to opt7.
> 
> OK. I wonder if we're approaching churn here, but I don't have a strong
> feeling.
> 
>> I think I can do without [1/2], by the way.
>>
>>  - die_for_incompatible_optN() (2 <= N <= 4) will keep accepting N
>>    pairs of <int, const char *>
>>
>>  - die_for_incompatible_opts() will take pairs of <int, const char *>,
>>    expects "int" to be 0 (not set), 1 (set), or EOF==-1 (sentinel).
>>
>>  - static inline void die_for_incompatible_opt2() emulation layer
>>    will call die_for_incompatible_opts(!!opt1, opt1_name, !!opt2,
>>    opt2_name, EOF).  Similarly for opt3() and opt4() variants.
> 
> Yeah, but then you can't get good compiler support, since I don't think
> there is an integer equivalent to LAST_ARG_MUST_BE_NULL. So the varargs
> interface feels less safe (and strictly worse since we are not actually
> helping any case that has more than 4 items).

You can still use LAST_ARG_MUST_BE_NULL if you require EOF _and_ NULL. Looks silly, but could be papered over with a macro:

#define die_for_incompatible_opts(...) \
	die_for_incompatible_opts_internal(__VA_ARGS__, EOF, NULL)

With such a macro you don't really need LAST_ARG_MUST_BE_NULL anymore, though, as it already guarantees termination by construction -- as long as the internal function is never called directly.

It's still less safe because it only checks the types of its first two arguments. On one hand this might suffice, because the rest of the arguments just need to continue the pattern. On the other hand it's error-handling code, which tends to be tested less, so a broken pattern might be overlooked.

Here's a type-safe variant, but it looks a bit odd with all those mustaches:

struct used_option {
	const char *name;
	bool used;
};

#define DIE_FOR_INCOMPATIBLE_OPTS(...) \
	die_for_incompatible_opts((struct used_option []){ \
		__VA_ARGS__, \
		{ NULL } \
	})

void die_for_incompatible_opts(const struct used_option *);
	
static inline void die_for_incompatible_opt4(int opt1, const char *opt1_name,
					     int opt2, const char *opt2_name,
					     int opt3, const char *opt3_name,
					     int opt4, const char *opt4_name)
{
	DIE_FOR_INCOMPATIBLE_OPTS({ opt1_name, opt1 },
				  { opt2_name, opt2 },
				  { opt3_name, opt3 },
				  { opt4_name, opt4 });
}

René