Re: [Outreachy kernel] [PATCH] staging: gdmwm: move constant to right side of comparison test

[Alison Schofield <amsfield22@gmail.com>]

On Sat, Oct 10, 2015 at 09:22:27AM +0200, Julia Lawall wrote:
> On Sat, 10 Oct 2015, Sudip Mukherjee wrote:
> 
> > On Thu, Oct 08, 2015 at 11:13:23PM -0700, Alison Schofield wrote:
> > > Move constant to right side of comparison test per checkpatch.pl:
> > > WARNING: Comparisons should place the constant on the right side of the
> > > test
> > > 
> > > Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> > > ---
> > >  drivers/staging/gdm72xx/gdm_wimax.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/staging/gdm72xx/gdm_wimax.c b/drivers/staging/gdm72xx/gdm_wimax.c
> > > index 6e8dbaf..69f00cc 100644
> > > --- a/drivers/staging/gdm72xx/gdm_wimax.c
> > > +++ b/drivers/staging/gdm72xx/gdm_wimax.c
> > > @@ -581,8 +581,8 @@ static int gdm_wimax_get_prepared_info(struct net_device *dev, char *buf,
> > >  		}
> > >  
> > >  		pos += gdm_wimax_hci_get_tlv(&buf[pos], &T, &L, &V);
> > > -		if (T == TLV_T(T_MAC_ADDRESS)) {
> > > -			if (L != dev->addr_len) {
> > > +		if (TLV_T(T_MAC_ADDRESS) == T) {
> > 
> > Is it correct? T and L are variables which are getting its values from
> > gdm_wimax_hci_get_tlv(). TLV_T is a macro which is ((x) & 0xff).
> > 
> > Do we use it like:
> > 
> > if ((T_MAC_ADDRESS & 0xff) == T)
> > 
> > or do we use like:
> > 
> > if (T == (T_MAC_ADDRESS & 0xff))
> 
> I'm not sure there is such a semantic intent to the rule.  Do something 
> complex and then see if it is like T, looks better to me than see if T is 
> like doing something complex.  The checkpatch/commit message is indeed 
> misleading, but the layout of characters looks better.
> 
> julia

Let me see if I have the correct understanding.

T and L are variables, not constants, therefore the checkpatch error and
my subsequent commit msg are misleading.  Regardless of whether this
case should have been flagged by checkpatch the edit improves
readability.

So, I'd lean towards keeping the edit and changing the commit msg & description like this:

	staging: gdmwm: move variable to right side of comparison test
    
	Move variables to right side of comparison test to improve readability.
		
Your thoughts?

alison