linux-kernel - Re: [PATCH] net: tulip: avoid unused variable warning

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 for Android: free password hash cracker in your pocket

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

Message-ID: <d863db0a-1740-45d5-b8de-746fa9d44fcb@gmx.de>
Date: Wed, 12 Mar 2025 15:08:35 +0100
From: Helge Deller <deller@....de>
To: Simon Horman <horms@...nel.org>, deller@...nel.org
Cc: netdev@...r.kernel.org, linux-parisc@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: tulip: avoid unused variable warning

On 3/12/25 14:14, Simon Horman wrote:
> On Sun, Mar 09, 2025 at 10:42:38PM +0100, deller@...nel.org wrote:
>> From: Helge Deller <deller@....de>
>>
>> When compiling with W=1 and CONFIG_TULIP_MWI=n one gets this warning:
>>   drivers/net/ethernet/dec/tulip/tulip_core.c: In function ‘tulip_init_one’:
>>   drivers/net/ethernet/dec/tulip/tulip_core.c:1309:22: warning: variable ‘force_csr0’ set but not used
>>
>> Avoid it by annotating the variable __maybe_unused, which seems to be
>> the easiest solution.
>>
>> Signed-off-by: Helge Deller <deller@....de>
>
> Hi Helge,
>
> A few thoughts on this:

Hi Simon,

Thanks for following up on this!

> Firstly, thanks for your patch, which I agree addresses the problem you
> have described.
>
> However, AFAIK, this is a rather old driver and I'm not sure that
> addressing somewhat cosmetic problems are worth the churn they cause:
> maybe it's best to leave it be.

Well, the only reason why I sent this patch is, because some people
are interested to get a Linux kernel build without any warnings when "W=1"
option is enabled.
This code in the tulip driver is one of the last 10 places in the kernel where
I see a warning at all, so I think it's worth fixing it, although it's just
cosmetic.


> But if we do want to fix this problem, I do wonder if the following
> solution, which leverages IS_ENABLED, is somehow nicer as
> it slightly reduces the amount of conditionally compiled code,
> thus increasing compile test coverage.

Full Ack from my side!
I wanted to keep my patch small, but your proposed patch is the better one.

I did not compile-test it, but if it builds you may add my:
Acked-by: Helge Deller <deller@....de>

Helge

> diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
> index 27e01d780cd0..75eac18ff246 100644
> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
> @@ -1177,7 +1177,6 @@ static void set_rx_mode(struct net_device *dev)
>   	iowrite32(csr6, ioaddr + CSR6);
>   }
>
> -#ifdef CONFIG_TULIP_MWI
>   static void tulip_mwi_config(struct pci_dev *pdev, struct net_device *dev)
>   {
>   	struct tulip_private *tp = netdev_priv(dev);
> @@ -1251,7 +1250,6 @@ static void tulip_mwi_config(struct pci_dev *pdev, struct net_device *dev)
>   		netdev_dbg(dev, "MWI config cacheline=%d, csr0=%08x\n",
>   			   cache, csr0);
>   }
> -#endif
>
>   /*
>    *	Chips that have the MRM/reserved bit quirk and the burst quirk. That
> @@ -1463,10 +1461,9 @@ static int tulip_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>
>   	INIT_WORK(&tp->media_work, tulip_tbl[tp->chip_id].media_task);
>
> -#ifdef CONFIG_TULIP_MWI
> -	if (!force_csr0 && (tp->flags & HAS_PCI_MWI))
> +	if (IS_ENABLED(CONFIG_TULIP_MWI) && !force_csr0 &&
> +	    (tp->flags & HAS_PCI_MWI))
>   		tulip_mwi_config (pdev, dev);
> -#endif
>
>   	/* Stop the chip's Tx and Rx processes. */
>   	tulip_stop_rxtx(tp);
>