GCC produces an unused variable warning for 'rc':
drivers/rapidio/rio_cm.c: In function ‘rio_txcq_handler’:
drivers/rapidio/rio_cm.c:673:7: warning: variable ‘rc’ set but
not used [-Wunused-but-set-variable]
The return value of rio_add_outb_message() is assigned to 'rc' but it
is never used. Fix this warning by logging an error if 'rc' is non-zero.
Signed-off-by: Anirudh Rayabharam <[email protected]>
---
Changes in v2:
Log an error instead of just removing 'rc'.
v1: https://lore.kernel.org/lkml/[email protected]/
---
drivers/rapidio/rio_cm.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/rapidio/rio_cm.c b/drivers/rapidio/rio_cm.c
index 50ec53d67a4c..971a36f1a23a 100644
--- a/drivers/rapidio/rio_cm.c
+++ b/drivers/rapidio/rio_cm.c
@@ -677,6 +677,11 @@ static void rio_txcq_handler(struct cm_dev *cm, int slot)
cm->tx_buf[cm->tx_slot] = req->buffer;
rc = rio_add_outb_message(cm->mport, req->rdev, cmbox,
req->buffer, req->len);
+ if (rc) {
+ riocm_error("Failed to add msg to tx queue (err=%d)",
+ rc);
+ }
+
kfree(req->buffer);
kfree(req);
--
2.26.2
On Thu, May 06, 2021 at 11:48:36PM +0530, Anirudh Rayabharam wrote:
> GCC produces an unused variable warning for 'rc':
>
> drivers/rapidio/rio_cm.c: In function ‘rio_txcq_handler’:
> drivers/rapidio/rio_cm.c:673:7: warning: variable ‘rc’ set but
> not used [-Wunused-but-set-variable]
>
> The return value of rio_add_outb_message() is assigned to 'rc' but it
> is never used. Fix this warning by logging an error if 'rc' is non-zero.
>
> Signed-off-by: Anirudh Rayabharam <[email protected]>
> ---
>
> Changes in v2:
> Log an error instead of just removing 'rc'.
>
> v1: https://lore.kernel.org/lkml/[email protected]/
>
> ---
> drivers/rapidio/rio_cm.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/rapidio/rio_cm.c b/drivers/rapidio/rio_cm.c
> index 50ec53d67a4c..971a36f1a23a 100644
> --- a/drivers/rapidio/rio_cm.c
> +++ b/drivers/rapidio/rio_cm.c
> @@ -677,6 +677,11 @@ static void rio_txcq_handler(struct cm_dev *cm, int slot)
> cm->tx_buf[cm->tx_slot] = req->buffer;
> rc = rio_add_outb_message(cm->mport, req->rdev, cmbox,
> req->buffer, req->len);
> + if (rc) {
> + riocm_error("Failed to add msg to tx queue (err=%d)",
> + rc);
That's pretty pointless (and no need for {}).
If an error happens, properly recover from it, don't just punt and tell
the user something bad happened and then ignore it.
thanks,
greg k-h
On Fri, May 07, 2021 at 08:39:21AM +0200, Greg KH wrote:
> On Thu, May 06, 2021 at 11:48:36PM +0530, Anirudh Rayabharam wrote:
> > GCC produces an unused variable warning for 'rc':
> >
> > drivers/rapidio/rio_cm.c: In function ‘rio_txcq_handler’:
> > drivers/rapidio/rio_cm.c:673:7: warning: variable ‘rc’ set but
> > not used [-Wunused-but-set-variable]
> >
> > The return value of rio_add_outb_message() is assigned to 'rc' but it
> > is never used. Fix this warning by logging an error if 'rc' is non-zero.
> >
> > Signed-off-by: Anirudh Rayabharam <[email protected]>
> > ---
> >
> > Changes in v2:
> > Log an error instead of just removing 'rc'.
> >
> > v1: https://lore.kernel.org/lkml/[email protected]/
> >
> > ---
> > drivers/rapidio/rio_cm.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/rapidio/rio_cm.c b/drivers/rapidio/rio_cm.c
> > index 50ec53d67a4c..971a36f1a23a 100644
> > --- a/drivers/rapidio/rio_cm.c
> > +++ b/drivers/rapidio/rio_cm.c
> > @@ -677,6 +677,11 @@ static void rio_txcq_handler(struct cm_dev *cm, int slot)
> > cm->tx_buf[cm->tx_slot] = req->buffer;
> > rc = rio_add_outb_message(cm->mport, req->rdev, cmbox,
> > req->buffer, req->len);
> > + if (rc) {
> > + riocm_error("Failed to add msg to tx queue (err=%d)",
> > + rc);
>
> That's pretty pointless (and no need for {}).
The point is that this fixes the compiler warning.
I used the {} because I had to split the statement into two lines and
thought it is more readable with the {}. checkpatch didn't complain
about it either.
>
> If an error happens, properly recover from it, don't just punt and tell
> the user something bad happened and then ignore it.
The primary motivation behind this patch is to fix the compiler warning.
This error is ignored even in the current tree. My patch at least logs it
so that people know that it's happening. And once they know that it's
happening and someone wants to propagate the error code to userspace or
handle it differently they can write a patch to do so.
Thanks!
- Anirudh.
On Fri, May 07, 2021 at 05:35:48PM +0530, Anirudh Rayabharam wrote:
> On Fri, May 07, 2021 at 08:39:21AM +0200, Greg KH wrote:
> > On Thu, May 06, 2021 at 11:48:36PM +0530, Anirudh Rayabharam wrote:
> > > GCC produces an unused variable warning for 'rc':
> > >
> > > drivers/rapidio/rio_cm.c: In function ‘rio_txcq_handler’:
> > > drivers/rapidio/rio_cm.c:673:7: warning: variable ‘rc’ set but
> > > not used [-Wunused-but-set-variable]
> > >
> > > The return value of rio_add_outb_message() is assigned to 'rc' but it
> > > is never used. Fix this warning by logging an error if 'rc' is non-zero.
> > >
> > > Signed-off-by: Anirudh Rayabharam <[email protected]>
> > > ---
> > >
> > > Changes in v2:
> > > Log an error instead of just removing 'rc'.
> > >
> > > v1: https://lore.kernel.org/lkml/[email protected]/
> > >
> > > ---
> > > drivers/rapidio/rio_cm.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/rapidio/rio_cm.c b/drivers/rapidio/rio_cm.c
> > > index 50ec53d67a4c..971a36f1a23a 100644
> > > --- a/drivers/rapidio/rio_cm.c
> > > +++ b/drivers/rapidio/rio_cm.c
> > > @@ -677,6 +677,11 @@ static void rio_txcq_handler(struct cm_dev *cm, int slot)
> > > cm->tx_buf[cm->tx_slot] = req->buffer;
> > > rc = rio_add_outb_message(cm->mport, req->rdev, cmbox,
> > > req->buffer, req->len);
> > > + if (rc) {
> > > + riocm_error("Failed to add msg to tx queue (err=%d)",
> > > + rc);
> >
> > That's pretty pointless (and no need for {}).
>
> The point is that this fixes the compiler warning.
Don't blindly fix warnings in ways to just shut a compiler up. Fix the
issue correctly please.
> > If an error happens, properly recover from it, don't just punt and tell
> > the user something bad happened and then ignore it.
>
> The primary motivation behind this patch is to fix the compiler warning.
> This error is ignored even in the current tree. My patch at least logs it
> so that people know that it's happening. And once they know that it's
> happening and someone wants to propagate the error code to userspace or
> handle it differently they can write a patch to do so.
Please write that patch, as-is, this change is pointless.
thanks,
greg k-h