linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] memstick: r592: ignore kfifo_out() return code again
@ 2021-04-21 13:51 Arnd Bergmann
  2021-04-21 17:16 ` Nick Desaulniers
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Arnd Bergmann @ 2021-04-21 13:51 UTC (permalink / raw)
  To: Maxim Levitsky, Alex Dubov, Ulf Hansson
  Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers,
	Jing Xiangfeng, Jiapeng Chong, linux-mmc, linux-kernel,
	clang-built-linux

From: Arnd Bergmann <arnd@arndb.de>

A minor cleanup to address a clang warning removed an assigned
but unused local variable, but this now caused a gcc warning as
kfifo_out() is annotated to require checking its return code:

In file included from drivers/memstick/host/r592.h:13,
                 from drivers/memstick/host/r592.c:21:
drivers/memstick/host/r592.c: In function 'r592_flush_fifo_write':
include/linux/kfifo.h:588:1: error: ignoring return value of '__kfifo_uint_must_check_helper' declared with attribute 'warn_unused_result' [-Werror=unused-result]
  588 | __kfifo_uint_must_check_helper( \
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  589 | ({ \
      | ~~~~
  590 |         typeof((fifo) + 1) __tmp = (fifo); \
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  591 |         typeof(__tmp->ptr) __buf = (buf); \
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  592 |         unsigned long __n = (n); \
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~
  593 |         const size_t __recsize = sizeof(*__tmp->rectype); \
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  594 |         struct __kfifo *__kfifo = &__tmp->kfifo; \
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  595 |         (__recsize) ?\
      |         ~~~~~~~~~~~~~~
  596 |         __kfifo_out_r(__kfifo, __buf, __n, __recsize) : \
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  597 |         __kfifo_out(__kfifo, __buf, __n); \
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  598 | }) \
      | ~~~~
  599 | )
      | ~
drivers/memstick/host/r592.c:367:9: note: in expansion of macro 'kfifo_out'
  367 |         kfifo_out(&dev->pio_fifo, buffer, 4);
      |         ^~~~~~~~~

The value was never checked here, and the purpose of the function
is only to flush the contents, so restore the old behavior but
add a cast to void and a comment, which hopefully warns with neither
gcc nor clang now.

If anyone has an idea for how to fix it without ignoring the return
code, that is probably better.

Fixes: 4b00ed3c5072 ("memstick: r592: remove unused variable")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/memstick/host/r592.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/memstick/host/r592.c b/drivers/memstick/host/r592.c
index 026fadaa1d5d..615a83782e55 100644
--- a/drivers/memstick/host/r592.c
+++ b/drivers/memstick/host/r592.c
@@ -359,12 +359,15 @@ static void r592_write_fifo_pio(struct r592_device *dev,
 /* Flushes the temporary FIFO used to make aligned DWORD writes */
 static void r592_flush_fifo_write(struct r592_device *dev)
 {
+	int ret;
 	u8 buffer[4] = { 0 };
 
 	if (kfifo_is_empty(&dev->pio_fifo))
 		return;
 
-	kfifo_out(&dev->pio_fifo, buffer, 4);
+	ret = kfifo_out(&dev->pio_fifo, buffer, 4);
+	/* intentionally ignore __must_check return code */
+	(void)ret;
 	r592_write_reg_raw_be(dev, R592_FIFO_PIO, *(u32 *)buffer);
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] memstick: r592: ignore kfifo_out() return code again
  2021-04-21 13:51 [PATCH] memstick: r592: ignore kfifo_out() return code again Arnd Bergmann
@ 2021-04-21 17:16 ` Nick Desaulniers
  2021-04-21 19:27   ` Arnd Bergmann
  2021-04-23  7:59 ` Ulf Hansson
  2021-04-26  9:44 ` Ulf Hansson
  2 siblings, 1 reply; 5+ messages in thread
From: Nick Desaulniers @ 2021-04-21 17:16 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Maxim Levitsky, Alex Dubov, Ulf Hansson, Arnd Bergmann,
	Nathan Chancellor, Jing Xiangfeng, Jiapeng Chong, linux-mmc,
	LKML, clang-built-linux

On Wed, Apr 21, 2021 at 6:52 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> A minor cleanup to address a clang warning removed an assigned
> but unused local variable, but this now caused a gcc warning as
> kfifo_out() is annotated to require checking its return code:
>
> In file included from drivers/memstick/host/r592.h:13,
>                  from drivers/memstick/host/r592.c:21:
> drivers/memstick/host/r592.c: In function 'r592_flush_fifo_write':
> include/linux/kfifo.h:588:1: error: ignoring return value of '__kfifo_uint_must_check_helper' declared with attribute 'warn_unused_result' [-Werror=unused-result]
>   588 | __kfifo_uint_must_check_helper( \
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   589 | ({ \
>       | ~~~~
>   590 |         typeof((fifo) + 1) __tmp = (fifo); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   591 |         typeof(__tmp->ptr) __buf = (buf); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   592 |         unsigned long __n = (n); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~
>   593 |         const size_t __recsize = sizeof(*__tmp->rectype); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   594 |         struct __kfifo *__kfifo = &__tmp->kfifo; \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   595 |         (__recsize) ?\
>       |         ~~~~~~~~~~~~~~
>   596 |         __kfifo_out_r(__kfifo, __buf, __n, __recsize) : \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   597 |         __kfifo_out(__kfifo, __buf, __n); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   598 | }) \
>       | ~~~~
>   599 | )
>       | ~
> drivers/memstick/host/r592.c:367:9: note: in expansion of macro 'kfifo_out'
>   367 |         kfifo_out(&dev->pio_fifo, buffer, 4);
>       |         ^~~~~~~~~
>
> The value was never checked here, and the purpose of the function
> is only to flush the contents, so restore the old behavior but
> add a cast to void and a comment, which hopefully warns with neither
> gcc nor clang now.
>
> If anyone has an idea for how to fix it without ignoring the return
> code, that is probably better.

Should r592_flush_fifo_write be made to return an int, then callers of
r592_flush_fifo_write percolate up their return code?
r592_transfer_fifo_pio() seems to only return 0, but its callers are
doing return code checking.

>
> Fixes: 4b00ed3c5072 ("memstick: r592: remove unused variable")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/memstick/host/r592.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/memstick/host/r592.c b/drivers/memstick/host/r592.c
> index 026fadaa1d5d..615a83782e55 100644
> --- a/drivers/memstick/host/r592.c
> +++ b/drivers/memstick/host/r592.c
> @@ -359,12 +359,15 @@ static void r592_write_fifo_pio(struct r592_device *dev,
>  /* Flushes the temporary FIFO used to make aligned DWORD writes */
>  static void r592_flush_fifo_write(struct r592_device *dev)
>  {
> +       int ret;
>         u8 buffer[4] = { 0 };
>
>         if (kfifo_is_empty(&dev->pio_fifo))
>                 return;
>
> -       kfifo_out(&dev->pio_fifo, buffer, 4);
> +       ret = kfifo_out(&dev->pio_fifo, buffer, 4);
> +       /* intentionally ignore __must_check return code */
> +       (void)ret;
>         r592_write_reg_raw_be(dev, R592_FIFO_PIO, *(u32 *)buffer);
>  }
>
> --
> 2.29.2
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] memstick: r592: ignore kfifo_out() return code again
  2021-04-21 17:16 ` Nick Desaulniers
@ 2021-04-21 19:27   ` Arnd Bergmann
  0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2021-04-21 19:27 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Maxim Levitsky, Alex Dubov, Ulf Hansson, Nathan Chancellor,
	Jing Xiangfeng, Jiapeng Chong, linux-mmc, LKML,
	clang-built-linux

On Wed, Apr 21, 2021 at 7:17 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
> On Wed, Apr 21, 2021 at 6:52 AM Arnd Bergmann <arnd@kernel.org> wrote:

> > The value was never checked here, and the purpose of the function
> > is only to flush the contents, so restore the old behavior but
> > add a cast to void and a comment, which hopefully warns with neither
> > gcc nor clang now.
> >
> > If anyone has an idea for how to fix it without ignoring the return
> > code, that is probably better.
>
> Should r592_flush_fifo_write be made to return an int, then callers of
> r592_flush_fifo_write percolate up their return code?
> r592_transfer_fifo_pio() seems to only return 0, but its callers are
> doing return code checking.

I looked into that but couldn't come up with anything useful to do with
the return code in the callers. kfifo_out() itself doesn't seem to
ever return an error but only the length.

        Arnd

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] memstick: r592: ignore kfifo_out() return code again
  2021-04-21 13:51 [PATCH] memstick: r592: ignore kfifo_out() return code again Arnd Bergmann
  2021-04-21 17:16 ` Nick Desaulniers
@ 2021-04-23  7:59 ` Ulf Hansson
  2021-04-26  9:44 ` Ulf Hansson
  2 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2021-04-23  7:59 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Maxim Levitsky, Alex Dubov, Arnd Bergmann, Nathan Chancellor,
	Nick Desaulniers, Jing Xiangfeng, Jiapeng Chong, linux-mmc,
	Linux Kernel Mailing List, clang-built-linux

On Wed, 21 Apr 2021 at 15:52, Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> A minor cleanup to address a clang warning removed an assigned
> but unused local variable, but this now caused a gcc warning as
> kfifo_out() is annotated to require checking its return code:
>
> In file included from drivers/memstick/host/r592.h:13,
>                  from drivers/memstick/host/r592.c:21:
> drivers/memstick/host/r592.c: In function 'r592_flush_fifo_write':
> include/linux/kfifo.h:588:1: error: ignoring return value of '__kfifo_uint_must_check_helper' declared with attribute 'warn_unused_result' [-Werror=unused-result]
>   588 | __kfifo_uint_must_check_helper( \
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   589 | ({ \
>       | ~~~~
>   590 |         typeof((fifo) + 1) __tmp = (fifo); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   591 |         typeof(__tmp->ptr) __buf = (buf); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   592 |         unsigned long __n = (n); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~
>   593 |         const size_t __recsize = sizeof(*__tmp->rectype); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   594 |         struct __kfifo *__kfifo = &__tmp->kfifo; \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   595 |         (__recsize) ?\
>       |         ~~~~~~~~~~~~~~
>   596 |         __kfifo_out_r(__kfifo, __buf, __n, __recsize) : \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   597 |         __kfifo_out(__kfifo, __buf, __n); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   598 | }) \
>       | ~~~~
>   599 | )
>       | ~
> drivers/memstick/host/r592.c:367:9: note: in expansion of macro 'kfifo_out'
>   367 |         kfifo_out(&dev->pio_fifo, buffer, 4);
>       |         ^~~~~~~~~
>
> The value was never checked here, and the purpose of the function
> is only to flush the contents, so restore the old behavior but
> add a cast to void and a comment, which hopefully warns with neither
> gcc nor clang now.
>
> If anyone has an idea for how to fix it without ignoring the return
> code, that is probably better.

Perhaps, if you can't do anything with return value, why is kfifo_out
declared like this?

>
> Fixes: 4b00ed3c5072 ("memstick: r592: remove unused variable")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Kind regards
Uffe

> ---
>  drivers/memstick/host/r592.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/memstick/host/r592.c b/drivers/memstick/host/r592.c
> index 026fadaa1d5d..615a83782e55 100644
> --- a/drivers/memstick/host/r592.c
> +++ b/drivers/memstick/host/r592.c
> @@ -359,12 +359,15 @@ static void r592_write_fifo_pio(struct r592_device *dev,
>  /* Flushes the temporary FIFO used to make aligned DWORD writes */
>  static void r592_flush_fifo_write(struct r592_device *dev)
>  {
> +       int ret;
>         u8 buffer[4] = { 0 };
>
>         if (kfifo_is_empty(&dev->pio_fifo))
>                 return;
>
> -       kfifo_out(&dev->pio_fifo, buffer, 4);
> +       ret = kfifo_out(&dev->pio_fifo, buffer, 4);
> +       /* intentionally ignore __must_check return code */
> +       (void)ret;
>         r592_write_reg_raw_be(dev, R592_FIFO_PIO, *(u32 *)buffer);
>  }
>
> --
> 2.29.2
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] memstick: r592: ignore kfifo_out() return code again
  2021-04-21 13:51 [PATCH] memstick: r592: ignore kfifo_out() return code again Arnd Bergmann
  2021-04-21 17:16 ` Nick Desaulniers
  2021-04-23  7:59 ` Ulf Hansson
@ 2021-04-26  9:44 ` Ulf Hansson
  2 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2021-04-26  9:44 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Maxim Levitsky, Alex Dubov, Arnd Bergmann, Nathan Chancellor,
	Nick Desaulniers, Jing Xiangfeng, Jiapeng Chong, linux-mmc,
	Linux Kernel Mailing List, clang-built-linux

On Wed, 21 Apr 2021 at 15:52, Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> A minor cleanup to address a clang warning removed an assigned
> but unused local variable, but this now caused a gcc warning as
> kfifo_out() is annotated to require checking its return code:
>
> In file included from drivers/memstick/host/r592.h:13,
>                  from drivers/memstick/host/r592.c:21:
> drivers/memstick/host/r592.c: In function 'r592_flush_fifo_write':
> include/linux/kfifo.h:588:1: error: ignoring return value of '__kfifo_uint_must_check_helper' declared with attribute 'warn_unused_result' [-Werror=unused-result]
>   588 | __kfifo_uint_must_check_helper( \
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   589 | ({ \
>       | ~~~~
>   590 |         typeof((fifo) + 1) __tmp = (fifo); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   591 |         typeof(__tmp->ptr) __buf = (buf); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   592 |         unsigned long __n = (n); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~
>   593 |         const size_t __recsize = sizeof(*__tmp->rectype); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   594 |         struct __kfifo *__kfifo = &__tmp->kfifo; \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   595 |         (__recsize) ?\
>       |         ~~~~~~~~~~~~~~
>   596 |         __kfifo_out_r(__kfifo, __buf, __n, __recsize) : \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   597 |         __kfifo_out(__kfifo, __buf, __n); \
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   598 | }) \
>       | ~~~~
>   599 | )
>       | ~
> drivers/memstick/host/r592.c:367:9: note: in expansion of macro 'kfifo_out'
>   367 |         kfifo_out(&dev->pio_fifo, buffer, 4);
>       |         ^~~~~~~~~
>
> The value was never checked here, and the purpose of the function
> is only to flush the contents, so restore the old behavior but
> add a cast to void and a comment, which hopefully warns with neither
> gcc nor clang now.
>
> If anyone has an idea for how to fix it without ignoring the return
> code, that is probably better.
>
> Fixes: 4b00ed3c5072 ("memstick: r592: remove unused variable")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied for next, thanks!

Kind regards
Uffe

> ---
>  drivers/memstick/host/r592.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/memstick/host/r592.c b/drivers/memstick/host/r592.c
> index 026fadaa1d5d..615a83782e55 100644
> --- a/drivers/memstick/host/r592.c
> +++ b/drivers/memstick/host/r592.c
> @@ -359,12 +359,15 @@ static void r592_write_fifo_pio(struct r592_device *dev,
>  /* Flushes the temporary FIFO used to make aligned DWORD writes */
>  static void r592_flush_fifo_write(struct r592_device *dev)
>  {
> +       int ret;
>         u8 buffer[4] = { 0 };
>
>         if (kfifo_is_empty(&dev->pio_fifo))
>                 return;
>
> -       kfifo_out(&dev->pio_fifo, buffer, 4);
> +       ret = kfifo_out(&dev->pio_fifo, buffer, 4);
> +       /* intentionally ignore __must_check return code */
> +       (void)ret;
>         r592_write_reg_raw_be(dev, R592_FIFO_PIO, *(u32 *)buffer);
>  }
>
> --
> 2.29.2
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-04-26  9:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21 13:51 [PATCH] memstick: r592: ignore kfifo_out() return code again Arnd Bergmann
2021-04-21 17:16 ` Nick Desaulniers
2021-04-21 19:27   ` Arnd Bergmann
2021-04-23  7:59 ` Ulf Hansson
2021-04-26  9:44 ` Ulf Hansson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).