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] [day] [month] [year] [list]

Message-ID: <YJUvuEYDbAWUb27y@kroah.com>
Date:   Fri, 7 May 2021 14:16:56 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Anirudh Rayabharam <mail@...rudhrb.com>
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 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 <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.

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