linux-kernel - Re: [PATCH 2/2] kconfig: do not imply the type of choice from the first entry

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] [day] [month] [year] [list]

Message-ID: <ZjnqDCqRLWmvFxWN@buildd.core.avm.de>
Date: Tue, 7 May 2024 10:45:00 +0200
From: Nicolas Schier <nicolas@...sle.eu>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
	Matt Porter <mporter@...nel.crashing.org>,
	Alexandre Bounine <alex.bou9@...il.com>,
	Jonathan Corbet <corbet@....net>,
	Nathan Chancellor <nathan@...nel.org>, linux-doc@...r.kernel.org
Subject: Re: [PATCH 2/2] kconfig: do not imply the type of choice from the
 first entry

On Sat, Apr 27, 2024 at 07:42:31PM +0900, Masahiro Yamada wrote:
> The followng two test cases are very similar, but the latter does not
> work.
> 
> [test case 1]
> 
>     choice
>             prompt "choose"
> 
>     config A
>             bool "A"
> 
>     if y
>     config B
>             bool "B"
>     endif
> 
>     endchoice
> 
> [test case 2]
> 
>     choice
>             prompt "choose"
> 
>     if y
>     config A
>             bool "A"
> 
>     config B
>             bool "B"
>     endif
> 
>     endchoice
> 
> Since 'if y' is always true, both of them should be equivalent to:
> 
>     choice
>             prompt "choose"
> 
>     config A
>             bool "A"
> 
>     config B
>             bool "B"
> 
>     endchoice
> 
> However, the test case 2 warns:
>   Kconfig:1:warning: config symbol defined without type
> 
> If the type of choice is not specified, it is implied from the first
> entry within the choice block.
> 
> When inferring the choice type, menu_finalize() checks only direct
> children of the choice. At this point, the menu entries still exist
> under the 'if' entry:
> 
>   choice
>   \-- if y
>       |-- A
>       \-- B
> 
> Later, menu_finalize() re-parents the menu, so A and B will be lifted up
> right under the choice:
> 
>   choice
>   |-- if y
>   |-- A
>   \-- B
> 
> This is complex because menu_finalize() sets attributes, restructures
> the menu tree, and checks the sanity at the same time, leading to some
> bugs.
> 
> It would be possible to resolve it by setting the choice type after
> re-parenting, but the current mechanism looks questionable to me.
> 
> Let's default all choices to 'bool' unless the type is specified.
> This change makes sense because 99% of choice use cases are bool.
> 
> There exists only one 'tristate' choice in drivers/rapidio/Kconfig.
> Another (much cleaner) approach would be to remove the tristate choice
> support entirely, but I have not yet made up my mind.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@...nel.org>
> ---

Thanks, looks good to me.

Reviewed-by: Nicolas Schier <n.schier@....de>