linux-kernel - Re: [PATCH] hwmon: dell-smm-hwmon: remove 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20210920123131.6kpv3ffxvm7xeqga@pali>
Date:   Mon, 20 Sep 2021 14:31:31 +0200
From:   Pali Rohár <pali@...nel.org>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     Jean Delvare <jdelvare@...e.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Armin Wolf <W_Armin@....de>, Arnd Bergmann <arnd@...db.de>,
        Carlos Alberto Lopez Perez <clopez@...lia.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Sebastian Oechsle <setboolean@...oud.com>,
        linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon: dell-smm-hwmon: remove unused variable warning

Hello!

On Monday 20 September 2021 14:14:16 Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
> 
> When procfs is disabled

... then the i8k_init_procfs function should not be called as the
purpose of I8K code / config option is to export functionality over
procfs. So when procfs is disabled then this i8k is noop.

Patch which do not allow compilation of I8K when procfs is not enabled
is pending here:

https://lore.kernel.org/linux-hwmon/20210910071921.16777-1-rdunlap@infradead.org/

Ideally please test or review it. As you are not the first one who
spotted -Werror problems with i8k and tried to workaround it.

https://lore.kernel.org/linux-hwmon/20210915151759.cxcbzxd74weg4qw6@pali/

For compatibility reasons I still have I8K enabled, so I have not
triggered this issue yet.

Anyway, do you know if somebody on desktop / laptop (which is the only
option where this i8k driver makes sense to be enabled) really using
kernel without procfs? I would like to know if this warning / error is
just artificial configuration generated by test scripts (and cannot be
hit by any user) or if there is a real user who will be affected by this
issue.

> the code produces a warning
> for an unused variable:
> 
> drivers/hwmon/dell-smm-hwmon.c: In function 'i8k_init_procfs':
> drivers/hwmon/dell-smm-hwmon.c:624:31: error: unused variable 'data' [-Werror=unused-variable]
>   624 |         struct dell_smm_data *data = dev_get_drvdata(dev);
>       |                               ^~~~

I'm starting to hate this -Werror decision... but seems that we have to
deal with it and together cleanup code as much as possible.

> Remove that local variable and just pass dev_get_drvdata(dev)
> directly.
> 
> Fixes: ba04d73c26ed ("hwmon: (dell-smm-hwmon) Move variables into a driver private data structure")
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  drivers/hwmon/dell-smm-hwmon.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 774c1b0715d9..0a3ce22c78e6 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -621,10 +621,8 @@ static void i8k_exit_procfs(void *param)
>  
>  static void __init i8k_init_procfs(struct device *dev)
>  {
> -	struct dell_smm_data *data = dev_get_drvdata(dev);
> -
>  	/* Register the proc entry */
> -	proc_create_data("i8k", 0, NULL, &i8k_proc_ops, data);
> +	proc_create_data("i8k", 0, NULL, &i8k_proc_ops, dev_get_drvdata(dev));
>  
>  	devm_add_action_or_reset(dev, i8k_exit_procfs, NULL);
>  }
> -- 
> 2.29.2
>