2021-07-26 20:20:43

by Bill Wendling

[permalink] [raw]
Subject: [PATCH v2 1/3] base: mark 'no_warn' as unused

Fix the following build warning:

drivers/base/module.c:36:6: error: variable 'no_warn' set but not used [-Werror,-Wunused-but-set-variable]
int no_warn;

This variable is used to remove another warning, but causes a warning
itself. Mark it as 'unused' to avoid that.

Signed-off-by: Bill Wendling <[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.32.0.432.gabb21c7263-goog


2021-07-26 20:48:51

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] base: mark 'no_warn' as unused

+ Greg and Rafael as the maintainer and reviewer of
drivers/base/module.c respectively, drop everyone else.

Original post:

https://lore.kernel.org/r/[email protected]/

On 7/26/2021 1:19 PM, 'Bill Wendling' via Clang Built Linux wrote:
> Fix the following build warning:
>
> drivers/base/module.c:36:6: error: variable 'no_warn' set but not used [-Werror,-Wunused-but-set-variable]
> int no_warn;
>
> This variable is used to remove another warning, but causes a warning
> itself. Mark it as 'unused' to avoid that.
>
> Signed-off-by: Bill Wendling <[email protected]>

Even though they evaluate to the same thing, it might be worth using
"__always_unused" here because it is :)

Regardless:

Reviewed-by: Nathan Chancellor <[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)
>

2021-07-26 21:04:01

by Bill Wendling

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] base: mark 'no_warn' as unused

On Mon, Jul 26, 2021 at 1:47 PM Nathan Chancellor <[email protected]> wrote:
>
> + Greg and Rafael as the maintainer and reviewer of
> drivers/base/module.c respectively, drop everyone else.
>
> Original post:
>
> https://lore.kernel.org/r/[email protected]/
>
> On 7/26/2021 1:19 PM, 'Bill Wendling' via Clang Built Linux wrote:
> > Fix the following build warning:
> >
> > drivers/base/module.c:36:6: error: variable 'no_warn' set but not used [-Werror,-Wunused-but-set-variable]
> > int no_warn;
> >
> > This variable is used to remove another warning, but causes a warning
> > itself. Mark it as 'unused' to avoid that.
> >
> > Signed-off-by: Bill Wendling <[email protected]>
>
> Even though they evaluate to the same thing, it might be worth using
> "__always_unused" here because it is :)
>
I thought about that, but went with the softer option in the (probably
very) unlikely event that it will be used in the future. :-) I'll be
happy to resubmit a new patch though.

-bw

> Regardless:
>
> Reviewed-by: Nathan Chancellor <[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)
> >

2021-07-27 05:28:26

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] base: mark 'no_warn' as unused

On Mon, Jul 26, 2021 at 01:47:33PM -0700, Nathan Chancellor wrote:
> + Greg and Rafael as the maintainer and reviewer of drivers/base/module.c
> respectively, drop everyone else.

Odd no one cc:ed us originally, I guess they didn't want the patch ever
merged? :(

>
> Original post:
>
> https://lore.kernel.org/r/[email protected]/
>
> On 7/26/2021 1:19 PM, 'Bill Wendling' via Clang Built Linux wrote:
> > Fix the following build warning:
> >
> > drivers/base/module.c:36:6: error: variable 'no_warn' set but not used [-Werror,-Wunused-but-set-variable]
> > int no_warn;

That's not going to be a good warning to ever have the kernel use due to
how lots of hardware works (i.e. we need to do a read after a write but
we can throw the read away as it does not matter).


> >
> > This variable is used to remove another warning, but causes a warning
> > itself. Mark it as 'unused' to avoid that.
> >
> > Signed-off-by: Bill Wendling <[email protected]>
>
> Even though they evaluate to the same thing, it might be worth using
> "__always_unused" here because it is :)

But it is not unused, the value is written into it.

So this isn't ok, sometimes we want to write to variables but never care
about the value, that does not mean the compiler should complain about
it.

thanks,

greg k-h

2021-07-27 06:18:31

by Bill Wendling

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] base: mark 'no_warn' as unused

On Mon, Jul 26, 2021 at 10:27 PM Greg Kroah-Hartman
<[email protected]> wrote:
> On Mon, Jul 26, 2021 at 01:47:33PM -0700, Nathan Chancellor wrote:
> > + Greg and Rafael as the maintainer and reviewer of drivers/base/module.c
> > respectively, drop everyone else.
>
> Odd no one cc:ed us originally, I guess they didn't want the patch ever
> merged? :(
>
> >
> > Original post:
> >
> > https://lore.kernel.org/r/[email protected]/
> >
> > On 7/26/2021 1:19 PM, 'Bill Wendling' via Clang Built Linux wrote:
> > > Fix the following build warning:
> > >
> > > drivers/base/module.c:36:6: error: variable 'no_warn' set but not used [-Werror,-Wunused-but-set-variable]
> > > int no_warn;
>
> That's not going to be a good warning to ever have the kernel use due to
> how lots of hardware works (i.e. we need to do a read after a write but
> we can throw the read away as it does not matter).
>
>
> > >
> > > This variable is used to remove another warning, but causes a warning
> > > itself. Mark it as 'unused' to avoid that.
> > >
> > > Signed-off-by: Bill Wendling <[email protected]>
> >
> > Even though they evaluate to the same thing, it might be worth using
> > "__always_unused" here because it is :)
>
> But it is not unused, the value is written into it.
>
I believe that only matters if the variable is marked "volatile".
Otherwise, the variable itself is never used. A "variable that's
written to but not read from," in fact, is the whole reason for the
warning.

> So this isn't ok, sometimes we want to write to variables but never care
> about the value, that does not mean the compiler should complain about
> it.
>
Typically, if you don't care about the return value, you simply don't
assign it to a variable (cf. printf). However, the functions that
assign to "no_warn" have the "warn_unused_result" attribute. The fact
that the variable is named "no_warn" seems to indicate that it's meant
to remain unused, even if it probably should be checked.

Would you rather the warning be turned off on some level?

-bw