linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Michal Marek <mmarek@suse.cz>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>,
	Jiri Slaby <jslaby@suse.cz>, Ingo Molnar <mingo@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] linux/kconfig.h: generalize IS_ENABLED logic
Date: Wed, 07 Oct 2015 23:33:25 +0200	[thread overview]
Message-ID: <87twq2pdne.fsf@rasmusvillemoes.dk> (raw)
In-Reply-To: <5614D49F.8070006@suse.cz> (Michal Marek's message of "Wed, 7 Oct 2015 10:15:27 +0200")

On Wed, Oct 07 2015, Michal Marek <mmarek@suse.cz> wrote:

> On 2015-10-06 23:05, Rasmus Villemoes wrote:
>> It's not hard to generalize the macro magic used to build the
>> IS_ENABLED macro and friends to produce a few other potentially useful
>> macros:
>> 
>> CHOOSE_EXPR(CONFIG_FOO, expr): if CONFIG_FOO is set expands to
>> expr, otherwise expands to nothing.
>> 
>> CHOOSE_EXPR(CONFIG_FOO, expr1, expr2): if CONFIG_FOO is set,
>> expands to expr1, otherwise expands to expr2.
>
> FWIW, I agree with Ingo that the CHOOSE_EXPR name is not really obvious.
> IF_CONFIG is a better alternative IMO, since the average programmer
> probably does not know __builtin_choose_expr() to see the analogy.

OK, CHOOSE_EXPR is out. But I think IF_CONFIG/COND_CONFIG might be a
little annoying or redundant, since "CONFIG" would also always be part
of the first argument.

Come to think of it, since this would be a primitive for conditional
compilation whose primary purpose is to eliminate the verbosity of
#ifdef/#endif, I'd prefer plain and simple COND, with COND_INITIALIZER
as a sidekick for that special purpose. Unfortunately, COND is already
used in a few places :( So I'll go with COND_CONFIG for now, but wait a
few days before sending v2, to see if anyone else has comments or naming
suggestions.

> While the C standard syntax requires struct-declaration to actually
> declare a member, the compiler will happily ignore the extra semicolon
> if you write
>
> truct task_struct {
>    ...
>    CHOOSE_EXPR(CONFIG_KASAN, unsigned int kasan_depth);
>    ...
> }
>
> So I think that the COND_DECLARATION macro is not necessary.

Thanks, I didn't know that. I see that both gcc and clang accept it
whether the extra semicolon is at the beginning or end of the struct,
and whether there's even a single actual member. OK, then
COND_DECLARATION is redundant (though it might still be useful as a
natural buddy to COND_INITIALIZER).

>> #define INIT_KASAN(tsk) COND_INITIALIZER(CONFIG_KASAN, .kasan_depth = 1)
>
> COND_INITIALIZER on the other hand is useful (CHOOSE_EXPR(CONFIG_KASAN,
> .kasan_depth = 1 _COMMA) does does not work, unfortunately).

Yeah, since we need to do the multiple expansion thing there's no way of
preventing _COMMA from expanding too early, so I'm pretty sure one would
need some specialized version of CHOOSE_EXPR (or whatever the name ends
up being). Also, I wouldn't really want users to have to supply the
_COMMA. One could also consider making COND_ARGUMENT an alias for it -
that could be useful for some seq_printf calls generating /proc files
(where the format string would be built with COND_CONFIG).

>> [and I'm certainly not proposing any mass conversion], but I think it
>> might be nice to avoid lots of short #ifdef/#else/#endif sections.
>
> It should be accompanied by a patch to scripts/tags.sh teaching
> ctags/etags about the new macros.

Do you mean that something like

	--regex-c='/COND_CONFIG\([^,]*,([^,]*)\)/\1/'

should be added so ctags would pick up the text in the true branch? I'm
not very familiar with ctags.

Thanks for the feedback, Michal and Ingo.

Rasmus

  reply	other threads:[~2015-10-07 21:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-06 12:29 [PATCH 1/2] x86: dumpstack, use pr_cont Jiri Slaby
2015-10-06 12:29 ` [PATCH 2/2] x86: text_poke, check if text_mutex is held Jiri Slaby
2015-10-06 16:00 ` [PATCH 1/2] x86: dumpstack, use pr_cont Rasmus Villemoes
2015-10-06 16:16   ` Jiri Slaby
2015-10-06 21:05     ` [PATCH 1/2] linux/kconfig.h: generalize IS_ENABLED logic Rasmus Villemoes
2015-10-06 21:05       ` [PATCH 2/2] x86: dumpstack: eliminate some #ifdefs Rasmus Villemoes
2015-10-07  7:03         ` Ingo Molnar
2016-03-26 20:40         ` [PATCH] x86: dumpstack: combine some printks Rasmus Villemoes
2016-04-01  6:37           ` [tip:x86/debug] x86/dumpstack: Combine some printk()s tip-bot for Rasmus Villemoes
2015-10-07  6:57       ` [PATCH 1/2] linux/kconfig.h: generalize IS_ENABLED logic Ingo Molnar
2015-10-07  8:15       ` Michal Marek
2015-10-07 21:33         ` Rasmus Villemoes [this message]
2015-10-08 11:40           ` Michal Marek
2015-11-16  8:00       ` using IS_ENABLED(CONFIG_xyz) effectively Vineet Gupta
2015-11-16  8:28         ` Geert Uytterhoeven
2015-11-16  8:35           ` Vineet Gupta
2015-11-16  8:52             ` Arnd Bergmann
2015-11-16 10:03               ` Vineet Gupta

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=87twq2pdne.fsf@rasmusvillemoes.dk \
    --to=linux@rasmusvillemoes.dk \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mmarek@suse.cz \
    --cc=paul.gortmaker@windriver.com \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).