[PATCH] driver base: slience unused warning

[PATCH] driver base: slience unused warning

[Su Hui]

Avoid unused warning with gcc and W=1 option.

drivers/base/module.c:36:6: error:
variable ‘no_warn’ set but not used [-Werror=unused-but-set-variable]

Signed-off-by: Su Hui <suhui@nfschina.com>
---
 drivers/base/module.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/module.c b/drivers/base/module.c
index 46ad4d636731..10494336d601 100644
--- a/drivers/base/module.c
+++ b/drivers/base/module.c
@@ -33,7 +33,7 @@ static void module_create_drivers_dir(struct module_kobject *mk)
 void module_add_driver(struct module *mod, struct device_driver *drv)
 {
 	char *driver_name;
-	int no_warn;
+	int __maybe_unused no_warn;
 	struct module_kobject *mk = NULL;
 
 	if (!drv)
-- 
2.30.2

Re: [PATCH] driver base: slience unused warning

[Greg KH]

On Thu, Aug 31, 2023 at 03:36:55PM +0800, Su Hui wrote:
> Avoid unused warning with gcc and W=1 option.
> 
> drivers/base/module.c:36:6: error:
> variable ‘no_warn’ set but not used [-Werror=unused-but-set-variable]
> 
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
>  drivers/base/module.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/module.c b/drivers/base/module.c
> index 46ad4d636731..10494336d601 100644
> --- a/drivers/base/module.c
> +++ b/drivers/base/module.c
> @@ -33,7 +33,7 @@ static void module_create_drivers_dir(struct module_kobject *mk)
>  void module_add_driver(struct module *mod, struct device_driver *drv)
>  {
>  	char *driver_name;
> -	int no_warn;
> +	int __maybe_unused no_warn;

But no_warn is being used in this file, it's being set but not read
which is ok.  That's a real use, so this change really isn't correct,
sorry.

greg k-h

Re: [PATCH] driver base: slience unused warning

[Dan Carpenter]

On Thu, Aug 31, 2023 at 03:36:55PM +0800, Su Hui wrote:
> Avoid unused warning with gcc and W=1 option.
> 
> drivers/base/module.c:36:6: error:
> variable ‘no_warn’ set but not used [-Werror=unused-but-set-variable]
> 
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
>  drivers/base/module.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/module.c b/drivers/base/module.c
> index 46ad4d636731..10494336d601 100644
> --- a/drivers/base/module.c
> +++ b/drivers/base/module.c
> @@ -33,7 +33,7 @@ static void module_create_drivers_dir(struct module_kobject *mk)
>  void module_add_driver(struct module *mod, struct device_driver *drv)
>  {
>  	char *driver_name;
> -	int no_warn;
> +	int __maybe_unused no_warn;

Just delete the variable if it isn't used.

regards,
dan carpenter

Re: [PATCH] driver base: slience unused warning

[Su Hui]

On 2023/9/7 18:49, Dan Carpenter wrote:
> On Thu, Aug 31, 2023 at 03:36:55PM +0800, Su Hui wrote:
>> Avoid unused warning with gcc and W=1 option.
>>
>> drivers/base/module.c:36:6: error:
>> variable ‘no_warn’ set but not used [-Werror=unused-but-set-variable]
>>
>> Signed-off-by: Su Hui <suhui@nfschina.com>
>> ---
>>   drivers/base/module.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/module.c b/drivers/base/module.c
>> index 46ad4d636731..10494336d601 100644
>> --- a/drivers/base/module.c
>> +++ b/drivers/base/module.c
>> @@ -33,7 +33,7 @@ static void module_create_drivers_dir(struct module_kobject *mk)
>>   void module_add_driver(struct module *mod, struct device_driver *drv)
>>   {
>>   	char *driver_name;
>> -	int no_warn;
>> +	int __maybe_unused no_warn;
> Just delete the variable if it isn't used.

Hi,

The variable "no_warn" is used to avoid warning like this:

drivers/base/module.c: In function ‘module_add_driver’:
drivers/base/module.c:62:2: error: ignoring return value of 
‘sysfs_create_link’ declared with attribute ‘warn_unused_result’ 
[-Werror=unused-result]

    62 |  sysfs_create_link(&drv->p->kobj, &mk->kobj, "module");
       |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This variable is been used but never be read, so gcc and W=1 give such 
warning.

drivers/base/module.c:36:6: error:
variable ‘no_warn’ set but not used [-Werror=unused-but-set-variable]

I wanted to use "__maybe_unused" to avoid  this warning.

However it seems like a wrong using of "__maybe_unused" as Greg KH said:

"But no_warn is being used in this file, it's being set but not read
which is ok.  That's a real use, so this change really isn't correct,
sorry."

Su Hui

>   
>
> regards,
> dan carpenter
>

Re: [PATCH] driver base: slience unused warning

[Maciej W. Rozycki]

On Fri, 8 Sep 2023, Su Hui wrote:

> This variable is been used but never be read, so gcc and W=1 give such
> warning.
> 
> drivers/base/module.c:36:6: error:
> variable ‘no_warn’ set but not used [-Werror=unused-but-set-variable]
> 
> I wanted to use "__maybe_unused" to avoid  this warning.
> 
> However it seems like a wrong using of "__maybe_unused" as Greg KH said:
> 
> "But no_warn is being used in this file, it's being set but not read
> which is ok.  That's a real use, so this change really isn't correct,
> sorry."

 The warning itself is a real issue to be sorted though.  Is this a use 
case for `#pragma GCC diagnostic'?

  Maciej

Re: [PATCH] driver base: slience unused warning

[Dan Carpenter]

On Wed, Oct 04, 2023 at 04:02:01PM +0100, Maciej W. Rozycki wrote:
> On Fri, 8 Sep 2023, Su Hui wrote:
> 
> > This variable is been used but never be read, so gcc and W=1 give such
> > warning.
> > 
> > drivers/base/module.c:36:6: error:
> > variable ‘no_warn’ set but not used [-Werror=unused-but-set-variable]
> > 
> > I wanted to use "__maybe_unused" to avoid  this warning.
> > 
> > However it seems like a wrong using of "__maybe_unused" as Greg KH said:
> > 
> > "But no_warn is being used in this file, it's being set but not read
> > which is ok.  That's a real use, so this change really isn't correct,
> > sorry."
> 
>  The warning itself is a real issue to be sorted though.  Is this a use 
> case for `#pragma GCC diagnostic'?

I thought Greg liked using __maybe_unused in this case?  This is
drivers/base.  Do the rest of us even get a vote?  ;)

If I do have a vote then #pragma is always the worst option.  Linus has
taught me to dislike pragmas a lot.

regards,
dan carpenter

Re: [PATCH] driver base: slience unused warning

[Maciej W. Rozycki]

On Wed, 4 Oct 2023, Dan Carpenter wrote:

> > > This variable is been used but never be read, so gcc and W=1 give such
> > > warning.
> > > 
> > > drivers/base/module.c:36:6: error:
> > > variable ‘no_warn’ set but not used [-Werror=unused-but-set-variable]
> > > 
> > > I wanted to use "__maybe_unused" to avoid  this warning.
> > > 
> > > However it seems like a wrong using of "__maybe_unused" as Greg KH said:
> > > 
> > > "But no_warn is being used in this file, it's being set but not read
> > > which is ok.  That's a real use, so this change really isn't correct,
> > > sorry."
> > 
> >  The warning itself is a real issue to be sorted though.  Is this a use 
> > case for `#pragma GCC diagnostic'?
> 
> I thought Greg liked using __maybe_unused in this case?  This is
> drivers/base.  Do the rest of us even get a vote?  ;)
> 
> If I do have a vote then #pragma is always the worst option.  Linus has
> taught me to dislike pragmas a lot.

 I'm not a great supporter of this solution, but at least it's guaranteed 
to work by definition.  Otherwise we could try to outsmart the compiler; 
perhaps:

	(no_warn = sysfs_create_link(&drv->p->kobj, &mk->kobj,
				     "module")), no_warn;

would work.  At the end of the day it's us who told the compiler to warn 
if the result of `sysfs_create_link' is unused with all the consequences.  
And while assigning to `no_warn' technically fulfils the requirement, the 
variable itself isn't used beyond being assigned to, which the compiler 
rightfully complains about because we asked for it.  It's up to us really 
to tell the compiler what we want it to complain about and what we do not.

  Maciej

Re: [PATCH] driver base: slience unused warning

[Dan Carpenter]

This is a W=1 static checker warning.  We've already reviewed it, and
marked it as old.  There isn't anything else required.

Or are we close to promoting the unused-but-set-variable warning from
W=1 to being on by default?  How many of these warnings are remaining?
It it's like only 20-50 warnings left then maybe we should consider the
other options but that kind of information needs to be in the cover
letter or otherwise we won't know about it.

regards,
dan carpenter

Re: [PATCH] driver base: slience unused warning

[Maciej W. Rozycki]

On Thu, 5 Oct 2023, Dan Carpenter wrote:

> This is a W=1 static checker warning.  We've already reviewed it, and
> marked it as old.  There isn't anything else required.

 Good point.

> Or are we close to promoting the unused-but-set-variable warning from
> W=1 to being on by default?  How many of these warnings are remaining?
> It it's like only 20-50 warnings left then maybe we should consider the
> other options but that kind of information needs to be in the cover
> letter or otherwise we won't know about it.

 Hmm, these warnings do help chasing dead code, which in turn may reveal 
real issues, such as where someone missed or forgot something when writing 
their code and a value that was supposed to be used somehow is instead 
discarded.

 Most commonly it will be the case when some code has been deliberately 
removed as it evolves and a part that is no longer needed has been missed 
by chance and left in place.  I've seen it happen.  Apart from the code 
sloppiness resulting it shouldn't matter that much though as the compiler 
is usually pretty good at discarding dead code.

  Maciej