linux-kernel - Re: [PATCH 1/2] linux/kconfig.h: generalize IS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 07 Oct 2015 23:33:25 +0200
From:	Rasmus Villemoes <linux@...musvillemoes.dk>
To:	Michal Marek <mmarek@...e.cz>
Cc:	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Jiri Slaby <jslaby@...e.cz>, Ingo Molnar <mingo@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] linux/kconfig.h: generalize IS_ENABLED logic

On Wed, Oct 07 2015, Michal Marek <mmarek@...e.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/