| Message ID | 20201104193549.4026187-35-lee.jones@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [01/36] tty: serdev: core: Remove unused variable 'dummy' | expand |
Le 04/11/2020 à 20:35, Lee Jones a écrit : > Fixes the following W=1 kernel build warning(s): > > drivers/tty/serial/pmac_zilog.h:365:58: warning: variable ‘garbage’ set but not used [-Wunused-but-set-variable] Explain how you are fixing this warning. Setting __always_unused is usually not the good solution for fixing this warning, but here I guess this is likely the good solution. But it should be explained why. Christophe > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Jiri Slaby <jirislaby@kernel.org> > Cc: Michael Ellerman <mpe@ellerman.id.au> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: linux-serial@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Signed-off-by: Lee Jones <lee.jones@linaro.org> > --- > drivers/tty/serial/pmac_zilog.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/pmac_zilog.h b/drivers/tty/serial/pmac_zilog.h > index bb874e76810e0..968aec7c1cf82 100644 > --- a/drivers/tty/serial/pmac_zilog.h > +++ b/drivers/tty/serial/pmac_zilog.h > @@ -362,7 +362,7 @@ static inline void zssync(struct uart_pmac_port *port) > > /* Misc macros */ > #define ZS_CLEARERR(port) (write_zsreg(port, 0, ERR_RES)) > -#define ZS_CLEARFIFO(port) do { volatile unsigned char garbage; \ > +#define ZS_CLEARFIFO(port) do { volatile unsigned char __always_unused garbage; \ > garbage = read_zsdata(port); \ > garbage = read_zsdata(port); \ > garbage = read_zsdata(port); \ >
On 05. 11. 20, 8:04, Christophe Leroy wrote: > > > Le 04/11/2020 à 20:35, Lee Jones a écrit : >> Fixes the following W=1 kernel build warning(s): >> >> drivers/tty/serial/pmac_zilog.h:365:58: warning: variable ‘garbage’ >> set but not used [-Wunused-but-set-variable] > > Explain how you are fixing this warning. > > Setting __always_unused is usually not the good solution for fixing > this warning, but here I guess this is likely the good solution. But it > should be explained why. Or, why is the "garbage =" needed in the first place? read_zsdata is not defined with __warn_unused_result__. And even if it was, would (void)!read_zsdata(port) fix it? >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: Jiri Slaby <jirislaby@kernel.org> >> Cc: Michael Ellerman <mpe@ellerman.id.au> >> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Cc: Paul Mackerras <paulus@samba.org> >> Cc: linux-serial@vger.kernel.org >> Cc: linuxppc-dev@lists.ozlabs.org >> Signed-off-by: Lee Jones <lee.jones@linaro.org> >> --- >> drivers/tty/serial/pmac_zilog.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/tty/serial/pmac_zilog.h >> b/drivers/tty/serial/pmac_zilog.h >> index bb874e76810e0..968aec7c1cf82 100644 >> --- a/drivers/tty/serial/pmac_zilog.h >> +++ b/drivers/tty/serial/pmac_zilog.h >> @@ -362,7 +362,7 @@ static inline void zssync(struct uart_pmac_port >> *port) >> /* Misc macros */ >> #define ZS_CLEARERR(port) (write_zsreg(port, 0, ERR_RES)) >> -#define ZS_CLEARFIFO(port) do { volatile unsigned char garbage; \ >> +#define ZS_CLEARFIFO(port) do { volatile unsigned char >> __always_unused garbage; \ >> garbage = read_zsdata(port); \ >> garbage = read_zsdata(port); \ >> garbage = read_zsdata(port); \ >> thanks, -- js
On Thu, 05 Nov 2020, Jiri Slaby wrote: > On 05. 11. 20, 8:04, Christophe Leroy wrote: > > > > > > Le 04/11/2020 à 20:35, Lee Jones a écrit : > > > Fixes the following W=1 kernel build warning(s): > > > > > > drivers/tty/serial/pmac_zilog.h:365:58: warning: variable > > > ‘garbage’ set but not used [-Wunused-but-set-variable] > > > > Explain how you are fixing this warning. > > > > Setting __always_unused is usually not the good solution for fixing > > this warning, but here I guess this is likely the good solution. But it > > should be explained why. There are normally 3 ways to fix this warning; - Start using/checking the variable/result - Remove the variable - Mark it as __{always,maybe}_unused The later just tells the compiler that not checking the resultant value is intentional. There are some functions (as Jiri mentions below) which are marked as '__must_check' which *require* a dummy (garbage) variable to be used. > Or, why is the "garbage =" needed in the first place? read_zsdata is not > defined with __warn_unused_result__. I used '__always_used' here for fear of breaking something. However, if it's safe to remove it, then all the better. > And even if it was, would (void)!read_zsdata(port) fix it? That's hideous. :D *Much* better to just use '__always_used' in that use-case. > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > Cc: Jiri Slaby <jirislaby@kernel.org> > > > Cc: Michael Ellerman <mpe@ellerman.id.au> > > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > > Cc: Paul Mackerras <paulus@samba.org> > > > Cc: linux-serial@vger.kernel.org > > > Cc: linuxppc-dev@lists.ozlabs.org > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > > --- > > > drivers/tty/serial/pmac_zilog.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/tty/serial/pmac_zilog.h > > > b/drivers/tty/serial/pmac_zilog.h > > > index bb874e76810e0..968aec7c1cf82 100644 > > > --- a/drivers/tty/serial/pmac_zilog.h > > > +++ b/drivers/tty/serial/pmac_zilog.h > > > @@ -362,7 +362,7 @@ static inline void zssync(struct uart_pmac_port > > > *port) > > > /* Misc macros */ > > > #define ZS_CLEARERR(port) (write_zsreg(port, 0, ERR_RES)) > > > -#define ZS_CLEARFIFO(port) do { volatile unsigned char garbage; \ > > > +#define ZS_CLEARFIFO(port) do { volatile unsigned char > > > __always_unused garbage; \ > > > garbage = read_zsdata(port); \ > > > garbage = read_zsdata(port); \ > > > garbage = read_zsdata(port); \ > > > > > thanks, -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
On 05. 11. 20, 9:36, Lee Jones wrote: > On Thu, 05 Nov 2020, Jiri Slaby wrote: > >> On 05. 11. 20, 8:04, Christophe Leroy wrote: >>> >>> >>> Le 04/11/2020 à 20:35, Lee Jones a écrit : >>>> Fixes the following W=1 kernel build warning(s): >>>> >>>> drivers/tty/serial/pmac_zilog.h:365:58: warning: variable >>>> ‘garbage’ set but not used [-Wunused-but-set-variable] >>> >>> Explain how you are fixing this warning. >>> >>> Setting __always_unused is usually not the good solution for fixing >>> this warning, but here I guess this is likely the good solution. But it >>> should be explained why. > > There are normally 3 ways to fix this warning; > > - Start using/checking the variable/result > - Remove the variable > - Mark it as __{always,maybe}_unused > > The later just tells the compiler that not checking the resultant > value is intentional. There are some functions (as Jiri mentions > below) which are marked as '__must_check' which *require* a dummy > (garbage) variable to be used. > >> Or, why is the "garbage =" needed in the first place? read_zsdata is not >> defined with __warn_unused_result__. > > I used '__always_used' here for fear of breaking something. > > However, if it's safe to remove it, then all the better. Yes please -- this "garbage" is one of the examples of volatile misuses. If readb didn't work on volatile pointer, marking the return variable as volatile wouldn't save it. >> And even if it was, would (void)!read_zsdata(port) fix it? > > That's hideous. :D Sure, marking reads as must_check would be insane. > *Much* better to just use '__always_used' in that use-case. Then using a dummy variable to fool must_check must mean must_check is used incorrectly, no :)? But there are always exceptions… thanks, -- js suse labs
On Thu, 05 Nov 2020, Jiri Slaby wrote: > On 05. 11. 20, 9:36, Lee Jones wrote: > > On Thu, 05 Nov 2020, Jiri Slaby wrote: > > > > > On 05. 11. 20, 8:04, Christophe Leroy wrote: > > > > > > > > > > > > Le 04/11/2020 à 20:35, Lee Jones a écrit : > > > > > Fixes the following W=1 kernel build warning(s): > > > > > > > > > > drivers/tty/serial/pmac_zilog.h:365:58: warning: variable > > > > > ‘garbage’ set but not used [-Wunused-but-set-variable] > > > > > > > > Explain how you are fixing this warning. > > > > > > > > Setting __always_unused is usually not the good solution for fixing > > > > this warning, but here I guess this is likely the good solution. But it > > > > should be explained why. > > > > There are normally 3 ways to fix this warning; > > > > - Start using/checking the variable/result > > - Remove the variable > > - Mark it as __{always,maybe}_unused > > > > The later just tells the compiler that not checking the resultant > > value is intentional. There are some functions (as Jiri mentions > > below) which are marked as '__must_check' which *require* a dummy > > (garbage) variable to be used. > > > > > Or, why is the "garbage =" needed in the first place? read_zsdata is not > > > defined with __warn_unused_result__. > > > > I used '__always_used' here for fear of breaking something. > > > > However, if it's safe to remove it, then all the better. > > Yes please -- this "garbage" is one of the examples of volatile misuses. If > readb didn't work on volatile pointer, marking the return variable as > volatile wouldn't save it. > > > > And even if it was, would (void)!read_zsdata(port) fix it? > > > > That's hideous. :D > > Sure, marking reads as must_check would be insane. > > > *Much* better to just use '__always_used' in that use-case. > > Then using a dummy variable to fool must_check must mean must_check is used > incorrectly, no :)? But there are always exceptions… Agreed on all points. Will fix. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
diff --git a/drivers/tty/serial/pmac_zilog.h b/drivers/tty/serial/pmac_zilog.h index bb874e76810e0..968aec7c1cf82 100644 --- a/drivers/tty/serial/pmac_zilog.h +++ b/drivers/tty/serial/pmac_zilog.h @@ -362,7 +362,7 @@ static inline void zssync(struct uart_pmac_port *port) /* Misc macros */ #define ZS_CLEARERR(port) (write_zsreg(port, 0, ERR_RES)) -#define ZS_CLEARFIFO(port) do { volatile unsigned char garbage; \ +#define ZS_CLEARFIFO(port) do { volatile unsigned char __always_unused garbage; \ garbage = read_zsdata(port); \ garbage = read_zsdata(port); \ garbage = read_zsdata(port); \
Fixes the following W=1 kernel build warning(s): drivers/tty/serial/pmac_zilog.h:365:58: warning: variable ‘garbage’ set but not used [-Wunused-but-set-variable] Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Jiri Slaby <jirislaby@kernel.org> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: linux-serial@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Lee Jones <lee.jones@linaro.org> --- drivers/tty/serial/pmac_zilog.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.25.1