| Message ID | 20210421135215.3414589-1-arnd@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | memstick: r592: ignore kfifo_out() return code again | expand |
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
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
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 >
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 >
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); }