All of lore.kernel.org
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] die_for_incompatible_opts(): accept more than four options
Date: Sat, 29 Aug 2026 20:04:21 +0200	[thread overview]
Message-ID: <5d5b1f26-192f-457d-bc18-499a3d7507fa@web.de> (raw)
In-Reply-To: <20260829111418.GA40814@coredump.intra.peff.net>

On 8/29/26 1:14 PM, Jeff King wrote:
> 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é


  parent reply	other threads:[~2026-08-29 18:04 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-26 23:31 [PATCH 0/2] die_for_incompatible_opts(): unbounded number of options Junio C Hamano
2026-08-26 23:31 ` [PATCH 1/2] die_for_incompatible_optN: swap the order of arguments Junio C Hamano
2026-08-26 23:31 ` [PATCH 2/2] die_for_incompatible_opts(): accept more than four options Junio C Hamano
2026-08-27  1:19   ` Elijah Newren
2026-08-27 14:22     ` Junio C Hamano
2026-08-27  4:55   ` Jeff King
2026-08-27 14:35     ` Junio C Hamano
2026-08-29 11:14       ` Jeff King
2026-08-29 17:51         ` Junio C Hamano
2026-08-29 18:04         ` René Scharfe [this message]
2026-08-27 17:28 ` [PATCH v2] die_for_incompatible_opts(): unbounded number of options Junio C Hamano
2026-08-29 11:15   ` Jeff King
2026-08-30 20:55     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5d5b1f26-192f-457d-bc18-499a3d7507fa@web.de \
    --to=l.s.r@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.