Re: [PATCH 20/24] staging: wilc1000: avoid line over 80 chars in tcp_process()

[Ajay Singh <ajay.kathat@microchip.com>]

Hi Dan,

On Mon, 27 Aug 2018 15:00:50 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> On Mon, Aug 27, 2018 at 10:54:38AM +0530, Ajay Singh wrote:
> > Hi Claudiu,
> > 
> > On Fri, 24 Aug 2018 12:31:28 +0300
> > Claudiu Beznea <Claudiu.Beznea@microchip.com> wrote:
> >   
> > > On 23.08.2018 13:33, Ajay Singh wrote:  
> > > > On Thu, 23 Aug 2018 11:12:08 +0300
> > > > Claudiu Beznea <Claudiu.Beznea@microchip.com> wrote:
> > > >     
> > > >> On 14.08.2018 09:50, Ajay Singh wrote:    
> > > >>> Cleanup patch to avoid line over 80 chars issue reported by
> > > >>> checkpatch.pl script.
> > > >>>
> > > >>> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> > > >>> ---
> > > >>>  drivers/staging/wilc1000/wilc_wlan.c | 7 ++++++-
> > > >>>  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >>>
> > > >>> diff --git a/drivers/staging/wilc1000/wilc_wlan.c
> > > >>> b/drivers/staging/wilc1000/wilc_wlan.c index 041c9dd..f0743d9
> > > >>> 100644 --- a/drivers/staging/wilc1000/wilc_wlan.c
> > > >>> +++ b/drivers/staging/wilc1000/wilc_wlan.c
> > > >>> @@ -137,6 +137,11 @@ static inline int
> > > >>> add_tcp_pending_ack(struct wilc_vif *vif, u32 ack, return 0;
> > > >>>  }
> > > >>>  
> > > >>> +static inline void clear_tcp_session_txq(struct wilc_vif
> > > >>> *vif, int index) +{
> > > >>> +	vif->ack_filter.pending_acks_info[index].txqe = NULL;
> > > >>> +}
> > > >>> +      
> > > >>
> > > >> This seems useless to me...    
> > > > 
> > > > Sorry, this point is not fully clear to me.
> > > > 
> > > > Did you mean setting of 'NULL' to
> > > > 'pending_acks_info[index].txqe' is not required?
> > > >     
> > > 
> > > No, having a new function that sets a variable just to avoid line
> > > over 80 warning.  
> > 
> > Okay got it.
> > How about using a temp variable to hold the value of
> > 'tqe->tcp_pending_ack_idx'. It can also help to overcome the
> > checkpatch warning.  
> 
> Just ignore the checkpatch warning...  Don't add a temporary variable
> just to please checkpatch...  It's good to fix checkpatch warnings if
> it makes the code cleaner, but I hate when people do:
> 
> 	tmp = some_long_variable_name;
> 	some_other_long_variable = tmp;
> 
> The tmp variable is only used one time and a simple statement becomes
> two statements and it breaks grep.
> 
> You could consider renaming some of the variables.  I think the
> "_info" doesn't add anything, because obviously it's information.
> Maybe "tcp_pending_ack_index" could just become "->ack_idx".

Thanks for providing the inputs. 
I will include the changes by using shorter name for
'tcp_pending_ack_index' in next version.

Regards,
Ajay