netdev - Re: [PATCH] s390/net: lcs: use IS_ENABLED() for kconfig detection

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: <ea55623d-d469-ddaf-92ce-3daf1d2d726f@infradead.org>
Date: Tue, 20 Jun 2023 19:35:17 -0700
From: Randy Dunlap <rdunlap@...radead.org>
To: linux-kernel@...r.kernel.org
Cc: Alexandra Winter <wintera@...ux.ibm.com>,
 Wenjia Zhang <wenjia@...ux.ibm.com>, linux-s390@...r.kernel.org,
 netdev@...r.kernel.org, Heiko Carstens <hca@...ux.ibm.com>,
 Vasily Gorbik <gor@...ux.ibm.com>, Alexander Gordeev
 <agordeev@...ux.ibm.com>, Christian Borntraeger <borntraeger@...ux.ibm.com>,
 Sven Schnelle <svens@...ux.ibm.com>
Subject: Re: [PATCH] s390/net: lcs: use IS_ENABLED() for kconfig detection

Hi,

On 6/15/23 15:21, Randy Dunlap wrote:
> When CONFIG_ETHERNET=m or CONFIG_FDDI=m, lcs.s has build errors or
> warnings:
> 
> ../drivers/s390/net/lcs.c:40:2: error: #error Cannot compile lcs.c without some net devices switched on.
>    40 | #error Cannot compile lcs.c without some net devices switched on.
> ../drivers/s390/net/lcs.c: In function 'lcs_startlan_auto':
> ../drivers/s390/net/lcs.c:1601:13: warning: unused variable 'rc' [-Wunused-variable]
>  1601 |         int rc;
> 
> Solve this by using IS_ENABLED(CONFIG_symbol) instead of ifdef
> CONFIG_symbol. The latter only works for builtin (=y) values
> while IS_ENABLED() works for builtin or modular values.
> 
> Signed-off-by: Randy Dunlap <rdunlap@...radead.org>
> Cc: Alexandra Winter <wintera@...ux.ibm.com>
> Cc: Wenjia Zhang <wenjia@...ux.ibm.com>
> Cc: linux-s390@...r.kernel.org
> Cc: netdev@...r.kernel.org
> Cc: Heiko Carstens <hca@...ux.ibm.com>
> Cc: Vasily Gorbik <gor@...ux.ibm.com>
> Cc: Alexander Gordeev <agordeev@...ux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@...ux.ibm.com>
> Cc: Sven Schnelle <svens@...ux.ibm.com>
> ---
>  drivers/s390/net/lcs.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff -- a/drivers/s390/net/lcs.c b/drivers/s390/net/lcs.c
> --- a/drivers/s390/net/lcs.c
> +++ b/drivers/s390/net/lcs.c
> @@ -36,7 +36,7 @@
>  #include "lcs.h"
>  
>  
> -#if !defined(CONFIG_ETHERNET) && !defined(CONFIG_FDDI)
> +#if !IS_ENABLED(CONFIG_ETHERNET) && !IS_ENABLED(CONFIG_FDDI)
>  #error Cannot compile lcs.c without some net devices switched on.
>  #endif
>  
> @@ -1601,14 +1601,14 @@ lcs_startlan_auto(struct lcs_card *card)
>  	int rc;
>  
>  	LCS_DBF_TEXT(2, trace, "strtauto");
> -#ifdef CONFIG_ETHERNET
> +#if IS_ENABLED(CONFIG_ETHERNET)
>  	card->lan_type = LCS_FRAME_TYPE_ENET;
>  	rc = lcs_send_startlan(card, LCS_INITIATOR_TCPIP);
>  	if (rc == 0)
>  		return 0;
>  
>  #endif
> -#ifdef CONFIG_FDDI
> +#if IS_ENABLED(CONFIG_FDDI)
>  	card->lan_type = LCS_FRAME_TYPE_FDDI;
>  	rc = lcs_send_startlan(card, LCS_INITIATOR_TCPIP);
>  	if (rc == 0)
> @@ -2139,13 +2139,13 @@ lcs_new_device(struct ccwgroup_device *c
>  		goto netdev_out;
>  	}
>  	switch (card->lan_type) {
> -#ifdef CONFIG_ETHERNET
> +#if IS_ENABLED(CONFIG_ETHERNET)
>  	case LCS_FRAME_TYPE_ENET:
>  		card->lan_type_trans = eth_type_trans;
>  		dev = alloc_etherdev(0);
>  		break;
>  #endif
> -#ifdef CONFIG_FDDI
> +#if IS_ENABLED(CONFIG_FDDI)
>  	case LCS_FRAME_TYPE_FDDI:
>  		card->lan_type_trans = fddi_type_trans;
>  		dev = alloc_fddidev(0);


kernel test robot reports build errors from this patch when
ETHERNET=y, FDDI=m, LCS=y:

  https://lore.kernel.org/all/202306202129.pl0AqK8G-lkp@intel.com/

Since the code before my patch expected (supported) FDDI=y only
(by checking for CONFIG_FDDI only and not checking for CONFIG_FDDI_MODULE),
the best solution that I can see is to enforce that expectation in
drivers/s390/net/Kconfig:

diff -- a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig
--- a/drivers/s390/net/Kconfig
+++ b/drivers/s390/net/Kconfig
@@ -5,7 +5,7 @@ menu "S/390 network device drivers"
 config LCS
 	def_tristate m
 	prompt "Lan Channel Station Interface"
-	depends on CCW && NETDEVICES && (ETHERNET || FDDI)
+	depends on CCW && NETDEVICES && (ETHERNET || FDDI = y)
 	help
 	  Select this option if you want to use LCS networking on IBM System z.
 	  This device driver supports FDDI (IEEE 802.7) and Ethernet.

What do people think of that change?
Any other ideas/suggestions?

thanks.
-- 
~Randy