linux-kernel - Re: [PATCH v2] rapidio: fix unused variable warning in rio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 May 2021 17:35:48 +0530
From:   Anirudh Rayabharam <mail@...rudhrb.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Matt Porter <mporter@...nel.crashing.org>,
        Alexandre Bounine <alex.bou9@...il.com>,
        linux-kernel-mentees@...ts.linuxfoundation.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] rapidio: fix unused variable warning in rio_cm.c

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 <mail@...rudhrb.com>
> > ---
> > 
> > Changes in v2:
> > Log an error instead of just removing 'rc'.
> > 
> > v1: https://lore.kernel.org/lkml/20210501055018.9244-1-mail@anirudhrb.com/
> > 
> > ---
> >  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.