linux-kernel - Re: [PATCH] staging: slicoss: remove unused variables

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]

Message-ID: <20150910132222.GA20312@sudip-pc>
Date:	Thu, 10 Sep 2015 18:52:22 +0530
From:	Sudip Mukherjee <sudipm.mukherjee@...il.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	Lior Dotan <liodot@...il.com>,
	Christopher Harrer <charrer@...critech.com>,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: slicoss: remove unused variables

On Wed, Sep 09, 2015 at 11:31:37AM -0700, Greg Kroah-Hartman wrote:
> On Fri, Sep 04, 2015 at 06:53:18PM +0530, Sudip Mukherjee wrote:
> > These variables were only assigned some values but they were never used.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@...torindia.org>
> > ---
> >  drivers/staging/slicoss/slicoss.c | 27 ++++++---------------------
> >  1 file changed, 6 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
> > index 8585970..1536ca0 100644
> > --- a/drivers/staging/slicoss/slicoss.c
> > +++ b/drivers/staging/slicoss/slicoss.c
<snip>
> > @@ -1730,15 +1727,13 @@ static void slic_link_event_handler(struct adapter *adapter)
> >  	pshmem = (struct slic_shmem *)(unsigned long)adapter->phys_shmem;
> >  
> >  #if BITS_PER_LONG == 64
> > -	status = slic_upr_request(adapter,
> > -				  SLIC_UPR_RLSR,
> > -				  SLIC_GET_ADDR_LOW(&pshmem->linkstatus),
> > -				  SLIC_GET_ADDR_HIGH(&pshmem->linkstatus),
> > -				  0, 0);
> > +	slic_upr_request(adapter, SLIC_UPR_RLSR,
> > +			 SLIC_GET_ADDR_LOW(&pshmem->linkstatus),
> > +			 SLIC_GET_ADDR_HIGH(&pshmem->linkstatus), 0, 0);
> >  #else
> > -	status = slic_upr_request(adapter, SLIC_UPR_RLSR,
> > -		(u32) &pshmem->linkstatus,	/* no 4GB wrap guaranteed */
> > -				  0, 0, 0);
> > +	slic_upr_request(adapter, SLIC_UPR_RLSR,
> > +			 (u32)&pshmem->linkstatus, /* no 4GB wrap guaranteed */
> > +			 0, 0, 0);
> 
> Shouldn't we do something with status instead of just ignoring it?
I can think of 3 possibilities.
1) Ignore it as this is writing READ_LINK_STATUS command to the device
asynchronously, and then writing UP configuration command. So if status
is error here then the device will not be UP.

2) loop here with a delay until the call succeeds. (will be a very bad
design, but there are some codes doing that). But this functions is also
called from an ISR so we should not be doing that.

3) return the error code and do the error handling properly by clearing
and releasing all resources acquired by the function which called it.

Which one will you suggest? I am sure you will say : 3.  :)

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/