staging: gdmwm: move constant to right side of comparison test
PATCH 1/1 staging: gdmwm: move constant to right side of comparison test
Move constant to right side of comparison test per checkpatch.pl:
WARNING: Comparisons should place the constant on the right side of the test
--- drivers/staging/gdm72xx/gdm_wimax.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/gdm72xx/gdm_wimax.c +2 -2
@@ -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 ) { +if (dev->addr_len !=L ) {netdev_err (dev,"%s Invalid information result T/L [%x/%d]\n" , __func__,T ,L );
Signature · 1 line
2.1.4
Quoted 14 lines · Alison Schofield
Move constant to right side of comparison test per checkpatch.pl:
WARNING: Comparisons should place the constant on the right side of the testSigned-off-by Alison Schofield <[email protected]>--- drivers/staging/gdm72xx/gdm_wimax.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/staging/gdm72xx/gdm_wimax.c +0
-0
drivers/staging/gdm72xx/gdm_wimax.c static int gdm_wimax_get_prepared_info(struct net_device *dev, char *buf, · line 584
Quoted 6 lines
drivers/staging/gdm72xx/gdm_wimax.c +1
-2@@ -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))
regards
sudip
Quoted 25 lines · Sudip Mukherjee
Quoted 14 lines · Alison Schofield
Move constant to right side of comparison test per checkpatch.pl:
WARNING: Comparisons should place the constant on the right side of the testSigned-off-by Alison Schofield <[email protected]>--- drivers/staging/gdm72xx/gdm_wimax.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/staging/gdm72xx/gdm_wimax.c +0
-0drivers/staging/gdm72xx/gdm_wimax.c static int gdm_wimax_get_prepared_info(struct net_device *dev, char *buf, · line 584
Quoted 6 lines
drivers/staging/gdm72xx/gdm_wimax.c +1
-2@@ -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_Tis a macro which is ((x) &0xff).
Quoted 7 lines
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
Quoted 33 lines · Julia Lawall
Quoted 31 lines · Sudip Mukherjee
Quoted 14 lines · Alison Schofield
Move constant to right side of comparison test per checkpatch.pl:
WARNING: Comparisons should place the constant on the right side of the testSigned-off-by Alison Schofield <[email protected]>--- drivers/staging/gdm72xx/gdm_wimax.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/staging/gdm72xx/gdm_wimax.c +0
-0drivers/staging/gdm72xx/gdm_wimax.c static int gdm_wimax_get_prepared_info(struct net_device *dev, char *buf, · line 584
Quoted 6 lines
drivers/staging/gdm72xx/gdm_wimax.c +1
-2@@ -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_Tis a macro which is ((x) &0xff).Do we use it like:
if ((
T_MAC_ADDRESS&0xff) == T)or do we use like:
Quoted 8 lines
Quoted 1 line
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
Quoted 41 lines · Alison Schofield
Quoted 40 lines · Julia Lawall
Quoted 25 lines · Sudip Mukherjee
Quoted 14 lines · Alison Schofield
Move constant to right side of comparison test per checkpatch.pl:
WARNING: Comparisons should place the constant on the right side of the testSigned-off-by Alison Schofield <[email protected]>--- drivers/staging/gdm72xx/gdm_wimax.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/staging/gdm72xx/gdm_wimax.c +0
-0drivers/staging/gdm72xx/gdm_wimax.c static int gdm_wimax_get_prepared_info(struct net_device *dev, char *buf, · line 584
Quoted 6 lines
drivers/staging/gdm72xx/gdm_wimax.c +1
-2@@ -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_Tis a macro which is ((x) &0xff).Quoted 7 lines
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.
Quoted 6 lines
Quoted 1 line
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.
Yes.
Quoted 4 lines
Regardless of whether this
case should have been flagged by checkpatch the edit improves readability.
Quoted 8 lines
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?
It seems good to me.
julia
Quoted 10 lines
alison
Signature · 5 lines
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151011063707.GA6423%40Ubuntu-D830. For more options, visit https://groups.google.com/d/optout.
Quoted 8 lines · Julia Lawall
Quoted 4 lines · Alison Schofield
Quoted 3 lines · Julia Lawall
Quoted 1 line · Sudip Mukherjee
On Thu, Oct 08, 2015 at 11:13:23PM -0700, Alison Schofield wrote:
<snip>
Quoted 22 lines
Quoted 6 lines
Quoted 6 lines
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.
Quoted 6 lines
Quoted 1 line
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.
Yes.
Quoted 7 lines
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:
Quoted 7 lines
Quoted 5 lines
staging: gdmwm: move variable to right side of comparison test
Move variables to right side of comparison test to improve readability.
Your thoughts?
It seems good to me.
Sorry for the late reply. Commit message looks good now.
regards
sudip
PATCH v2 1/1 staging: gdmwm: move variables to right side of comparison test
In reply to Alison SchofieldMove variables to right side of comparison test to improve readability.
---
Changes in v2: clarified commit message and description
drivers/staging/gdm72xx/gdm_wimax.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/gdm72xx/gdm_wimax.c +2 -2
@@ -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 ) { +if (dev->addr_len !=L ) {netdev_err (dev,"%s Invalid information result T/L [%x/%d]\n" , __func__,T ,L );
Signature · 1 line
2.1.4