Re: [PATCH] staging: vt6655: Remove useless else

[Marcos Antonio de Jesus Filho <mdejesusfilho@gmail.com>]

On Thu, Oct 29, 2020 at 01:09:45AM -0700, Marcos Antonio de Jesus Filho wrote:
> On Thu, Oct 29, 2020 at 08:07:23AM +0100, Greg Kroah-Hartman wrote:
> > On Wed, Oct 28, 2020 at 11:59:58PM -0700, Marcos Antonio de Jesus Filho wrote:
> > > The else statement is not useful due to the presence of a return
> > > statement on the if block. Remove the else statement and adjust the
> > > indentation of the code. Reported by checkpatch.
> > > 
> > > Signed-off-by: Marcos Antonio de Jesus Filho <mdejesusfilho@gmail.com>
> > > ---
> > >  drivers/staging/vt6655/rxtx.c | 36 +++++++++++++++++------------------
> > >  1 file changed, 18 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> > > index 477d19314634..978efe81077a 100644
> > > --- a/drivers/staging/vt6655/rxtx.c
> > > +++ b/drivers/staging/vt6655/rxtx.c
> > > @@ -519,28 +519,28 @@ s_uFillDataHead(
> > >  										wCurrentRate, bNeedAck, uFragIdx, cbLastFragmentSize, uMACfragNum, byFBOption));
> > >  			buf->time_stamp_off = vnt_time_stamp_off(pDevice, wCurrentRate);
> > >  			return buf->duration;
> > > -		} else {
> > > -			struct vnt_tx_datahead_ab *buf = pTxDataHead;
> > > -			/* Get SignalField, ServiceField & Length */
> > > -			vnt_get_phy_field(pDevice, cbFrameLength, wCurrentRate,
> > > -					  byPktType, &buf->ab);
> > > +		}
> > >  
> > > -			if (is_pspoll) {
> > > -				__le16 dur = cpu_to_le16(pDevice->current_aid | BIT(14) | BIT(15));
> > > +		struct vnt_tx_datahead_ab *buf = pTxDataHead;
> > > +		/* Get SignalField, ServiceField & Length */
> > > +		vnt_get_phy_field(pDevice, cbFrameLength, wCurrentRate,
> > > +				  byPktType, &buf->ab);
> > >  
> > > -				buf->duration = dur;
> > > -			} else {
> > > -				/* Get Duration and TimeStampOff */
> > > -				buf->duration =
> > > -					cpu_to_le16((u16)s_uGetDataDuration(pDevice, DATADUR_A, cbFrameLength, byPktType,
> > > -									    wCurrentRate, bNeedAck, uFragIdx,
> > > -									    cbLastFragmentSize, uMACfragNum,
> > > -									    byFBOption));
> > > -			}
> > > +		if (is_pspoll) {
> > > +			__le16 dur = cpu_to_le16(pDevice->current_aid | BIT(14) | BIT(15));
> > >  
> > > -			buf->time_stamp_off = vnt_time_stamp_off(pDevice, wCurrentRate);
> > > -			return buf->duration;
> > > +			buf->duration = dur;
> > > +		} else {
> > > +			/* Get Duration and TimeStampOff */
> > > +			buf->duration =
> > > +				cpu_to_le16((u16)s_uGetDataDuration(pDevice, DATADUR_A, cbFrameLength, byPktType,
> > > +								    wCurrentRate, bNeedAck, uFragIdx,
> > > +								    cbLastFragmentSize, uMACfragNum,
> > > +								    byFBOption));
> > >  		}
> > > +
> > > +		buf->time_stamp_off = vnt_time_stamp_off(pDevice, wCurrentRate);
> > > +		return buf->duration;
> > >  	} else {
> > 
> > You do the same thing here, right?  Shouldn't this else be dropped?
> 
> Definitely. I did not do it because we are instructed to solve one 'Warning' at time. I was going to do it in another patch.
> Should I edit this patch and fix it now?
>  
> > 
> > thanks,
> > 
> > greg k-h

I notice that after removing the else statement I get the compilation error "ISO C90 forbids mixed declarations and code", because we eliminate the scope of the else block. Should I leave as it is?