* [PATCH] staging: gdmwm: move constant to right side of comparison test
@ 2015-10-09 6:13 Alison Schofield
2015-10-10 7:00 ` [Outreachy kernel] " Sudip Mukherjee
2015-10-12 20:53 ` [PATCH v2] staging: gdmwm: move variables " Alison Schofield
0 siblings, 2 replies; 7+ messages in thread
From: Alison Schofield @ 2015-10-09 6:13 UTC (permalink / raw)
To: outreachy-kernel
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) {
+ if (dev->addr_len != L) {
netdev_err(dev,
"%s Invalid information result T/L [%x/%d]\n",
__func__, T, L);
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: gdmwm: move constant to right side of comparison test
2015-10-09 6:13 [PATCH] staging: gdmwm: move constant to right side of comparison test Alison Schofield
@ 2015-10-10 7:00 ` Sudip Mukherjee
2015-10-10 7:22 ` Julia Lawall
2015-10-12 20:53 ` [PATCH v2] staging: gdmwm: move variables " Alison Schofield
1 sibling, 1 reply; 7+ messages in thread
From: Sudip Mukherjee @ 2015-10-10 7:00 UTC (permalink / raw)
To: Alison Schofield; +Cc: outreachy-kernel
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))
regards
sudip
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: gdmwm: move constant to right side of comparison test
2015-10-10 7:00 ` [Outreachy kernel] " Sudip Mukherjee
@ 2015-10-10 7:22 ` Julia Lawall
2015-10-11 6:37 ` Alison Schofield
0 siblings, 1 reply; 7+ messages in thread
From: Julia Lawall @ 2015-10-10 7:22 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: Alison Schofield, outreachy-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: gdmwm: move constant to right side of comparison test
2015-10-10 7:22 ` Julia Lawall
@ 2015-10-11 6:37 ` Alison Schofield
2015-10-11 10:04 ` Julia Lawall
0 siblings, 1 reply; 7+ messages in thread
From: Alison Schofield @ 2015-10-11 6:37 UTC (permalink / raw)
To: Julia Lawall, Sudip Mukherjee; +Cc: outreachy-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: gdmwm: move constant to right side of comparison test
2015-10-11 6:37 ` Alison Schofield
@ 2015-10-11 10:04 ` Julia Lawall
2015-10-12 9:43 ` Sudip Mukherjee
0 siblings, 1 reply; 7+ messages in thread
From: Julia Lawall @ 2015-10-11 10:04 UTC (permalink / raw)
To: Alison Schofield; +Cc: Sudip Mukherjee, outreachy-kernel
On Sat, 10 Oct 2015, Alison Schofield wrote:
> 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.
Yes.
> 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?
It seems good to me.
julia
>
> alison
>
> --
> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: gdmwm: move constant to right side of comparison test
2015-10-11 10:04 ` Julia Lawall
@ 2015-10-12 9:43 ` Sudip Mukherjee
0 siblings, 0 replies; 7+ messages in thread
From: Sudip Mukherjee @ 2015-10-12 9:43 UTC (permalink / raw)
To: Julia Lawall; +Cc: Alison Schofield, outreachy-kernel
On Sun, Oct 11, 2015 at 12:04:22PM +0200, Julia Lawall wrote:
>
>
> On Sat, 10 Oct 2015, Alison Schofield wrote:
>
> > 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:
<snip>
> > >
> > > 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.
>
> Yes.
>
> > 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?
>
> It seems good to me.
Sorry for the late reply. Commit message looks good now.
regards
sudip
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] staging: gdmwm: move variables to right side of comparison test
2015-10-09 6:13 [PATCH] staging: gdmwm: move constant to right side of comparison test Alison Schofield
2015-10-10 7:00 ` [Outreachy kernel] " Sudip Mukherjee
@ 2015-10-12 20:53 ` Alison Schofield
1 sibling, 0 replies; 7+ messages in thread
From: Alison Schofield @ 2015-10-12 20:53 UTC (permalink / raw)
To: outreachy-kernel
Move variables to right side of comparison test to improve readability.
Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
Changes in v2: clarified commit message and description
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) {
+ if (dev->addr_len != L) {
netdev_err(dev,
"%s Invalid information result T/L [%x/%d]\n",
__func__, T, L);
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-10-12 20:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-09 6:13 [PATCH] staging: gdmwm: move constant to right side of comparison test Alison Schofield
2015-10-10 7:00 ` [Outreachy kernel] " Sudip Mukherjee
2015-10-10 7:22 ` Julia Lawall
2015-10-11 6:37 ` Alison Schofield
2015-10-11 10:04 ` Julia Lawall
2015-10-12 9:43 ` Sudip Mukherjee
2015-10-12 20:53 ` [PATCH v2] staging: gdmwm: move variables " Alison Schofield
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.