linux-kernel - Re: kernel coding style for if ... else which cross #ifdef

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]

Message-ID: <20080524104956.GD4886@cs181133002.pp.htv.fi>
Date:	Sat, 24 May 2008 13:49:56 +0300
From:	Adrian Bunk <bunk@...nel.org>
To:	Jeremy Fitzhardinge <jeremy@...p.org>
Cc:	Sam Ravnborg <sam@...nborg.org>, "H. Peter Anvin" <hpa@...or.com>,
	Steve French <smfrench@...il.com>,
	lkml <linux-kernel@...r.kernel.org>
Subject: Re: kernel coding style for if ... else which cross #ifdef

On Sat, May 24, 2008 at 11:06:21AM +0100, Jeremy Fitzhardinge wrote:
> Sam Ravnborg wrote:
>> On Fri, May 23, 2008 at 10:42:58PM -0700, H. Peter Anvin wrote:
>>   
>>> Sam Ravnborg wrote:
>>>     
>>>>> *However*, the best would really be if we changed Kconfig to emit 
>>>>> configuration constants what were 0/1 instead of 
>>>>> undefined/defined. That way we could do:
>>>>>
>>>>> 	if (CONFIG_SOMETHING && foo) {
>>>>> 		/* ... something ... */
>>>>> 	} else if ((mode & S_IWUGO) == 0) {
>>>>> 		/* ... */
>>>>>         
>>>> We could do that - but then it would need another
>>>> name not to clash with all the places where we rely
>>>> on CONFIG_FOO='n' => CONFIG_FOO is not defined.
>>>>
>>>> We could teach kconfig to emit something like:
>>>> #define KFOO 0   (for the 'n' value)
>>>> And 1 or 2 for the y and m values.
>>>>
>>>>       
>>> I don't think we want to use "1 or 2"... I suspect we want to use the 
>>> same booleans we currently have.
>>>     
>> I'm a bit dense (or I need more coffe - it's morning here).
>> What "same booleans"?
>>   
>
> They should be plain 0/1 booleans.  For a bool/tristate option FOO, it  
> would define:
>
> Enabled y:
>
>    #define CONFIG_FOO
>    #define CFG_FOO   1
>    #undef CONFIG_FOO_MODULE
>    #define CFG_FOO_MODULE 0
>
>
> Enabled m:
>
>    #define CONFIG_FOO
>    #define CFG_FOO   1
>    #define CONFIG_FOO_MODULE
>    #define CFG_FOO_MODULE 1
>...

A quite common pattern in the kernel is:
#if defined(CONFIG_FOO) || (defined(CONFIG_FOO_MODULE) && defined(MODULE))

Your suggestion would require them to be changed to:
#if (defined(CONFIG_FOO) && !defined(CONFIG_FOO_MODULE)) || (defined(CONFIG_FOO_MODULE) && defined(MODULE))

(We could push these cases to kconfig, but there might also be other 
 cases where changing the existing semantics of CONFIG_FOO could cause
 breakages.)

We see daily in kconfig that mixing tristates with bools is tricky 
(especially since bools are used with different intended semantics),
and I don't think doing the same in the source files would be an 
improvement.

We might be able to do (without any CFG_FOO_MODULE at all):

Enabled m:

    #undef CONFIG_FOO
    #define CFG_FOO   0
    #define CONFIG_FOO_MODULE

And let everyone who want's "either y or m" semantics to convert the 
tristate to a bool with this semantics in kconfig himself:

config FOO
	tristate

config BAR
	def_bool FOO

Especially since this is actually a relatively unusual (and not nice) 
case since usually adding a module does not (and should) not change the 
kernel image.

>    J

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--
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/