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 <[email protected]>
---
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
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 <[email protected]>
>> ---
>> 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
>
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
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
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
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
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