Re: [PATCH] memstick: r592: ignore kfifo_out() return code again

From: Nick Desaulniers
Date: Wed Apr 21 2021 - 13:17:15 EST


On Wed, Apr 21, 2021 at 6:52 AM Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
>
> From: Arnd Bergmann <arnd@xxxxxxxx>
>
> 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@xxxxxxxx>
> ---
> 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