linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] B43: misplaced parentheses?
@ 2009-02-15 16:37 Roel Kluin
  2009-02-15 16:46 ` Michael Buesch
  2009-02-15 17:05 ` Larry Finger
  0 siblings, 2 replies; 6+ messages in thread
From: Roel Kluin @ 2009-02-15 16:37 UTC (permalink / raw)
  To: stefano.brivio; +Cc: linux-wireless, Andrew Morton

I think below is what was intended? otherwise we could as well have written:

 	b43_radio_write16(dev, txctl_reg,
 			  (b43_radio_read16(dev, txctl_reg) & ~txctl_value)
			  || (rfatt->with_padmix) ? txctl_value : 0);

			  ^^--- Note: boolean or

please review.
-------------------------->8------------------8<---------------------------
Fix misplaced parentheses

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/net/wireless/b43/lo.c b/drivers/net/wireless/b43/lo.c
index 6a18a14..88ed75f 100644
--- a/drivers/net/wireless/b43/lo.c
+++ b/drivers/net/wireless/b43/lo.c
@@ -783,7 +783,7 @@ struct b43_lo_calib * b43_calibrate_lo_setting(struct b43_wldev *dev,
 			  | rfatt->att);
 	b43_radio_write16(dev, txctl_reg,
 			  (b43_radio_read16(dev, txctl_reg) & ~txctl_value)
-			  | (rfatt->with_padmix) ? txctl_value : 0);
+			  | (rfatt->with_padmix ? txctl_value : 0));
 
 	max_rx_gain = rfatt->att * 2;
 	max_rx_gain += bbatt->att / 2;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] B43: misplaced parentheses?
  2009-02-15 16:37 [PATCH] B43: misplaced parentheses? Roel Kluin
@ 2009-02-15 16:46 ` Michael Buesch
  2009-02-15 16:59   ` Michael Buesch
  2009-02-15 17:05 ` Larry Finger
  1 sibling, 1 reply; 6+ messages in thread
From: Michael Buesch @ 2009-02-15 16:46 UTC (permalink / raw)
  To: Roel Kluin; +Cc: stefano.brivio, linux-wireless, Andrew Morton

On Sunday 15 February 2009 17:37:59 Roel Kluin wrote:
> I think below is what was intended? otherwise we could as well have written:
> 
>  	b43_radio_write16(dev, txctl_reg,
>  			  (b43_radio_read16(dev, txctl_reg) & ~txctl_value)
> 			  || (rfatt->with_padmix) ? txctl_value : 0);
> 
> 			  ^^--- Note: boolean or

Uhm, no?
Can you please explain why you think this is equal? Does | have precedence over ?:

> 
> please review.
> -------------------------->8------------------8<---------------------------
> Fix misplaced parentheses
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/drivers/net/wireless/b43/lo.c b/drivers/net/wireless/b43/lo.c
> index 6a18a14..88ed75f 100644
> --- a/drivers/net/wireless/b43/lo.c
> +++ b/drivers/net/wireless/b43/lo.c
> @@ -783,7 +783,7 @@ struct b43_lo_calib * b43_calibrate_lo_setting(struct b43_wldev *dev,
>  			  | rfatt->att);
>  	b43_radio_write16(dev, txctl_reg,
>  			  (b43_radio_read16(dev, txctl_reg) & ~txctl_value)
> -			  | (rfatt->with_padmix) ? txctl_value : 0);
> +			  | (rfatt->with_padmix ? txctl_value : 0));
>  
>  	max_rx_gain = rfatt->att * 2;
>  	max_rx_gain += bbatt->att / 2;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 



-- 
Greetings, Michael.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] B43: misplaced parentheses?
  2009-02-15 16:46 ` Michael Buesch
@ 2009-02-15 16:59   ` Michael Buesch
  0 siblings, 0 replies; 6+ messages in thread
From: Michael Buesch @ 2009-02-15 16:59 UTC (permalink / raw)
  To: Roel Kluin; +Cc: stefano.brivio, linux-wireless, Andrew Morton

On Sunday 15 February 2009 17:46:44 Michael Buesch wrote:
> On Sunday 15 February 2009 17:37:59 Roel Kluin wrote:
> > I think below is what was intended? otherwise we could as well have written:
> > 
> >  	b43_radio_write16(dev, txctl_reg,
> >  			  (b43_radio_read16(dev, txctl_reg) & ~txctl_value)
> > 			  || (rfatt->with_padmix) ? txctl_value : 0);
> > 
> > 			  ^^--- Note: boolean or
> 
> Uhm, no?
> Can you please explain why you think this is equal? Does | have precedence over ?:

Hm, | does indeed seem to have precedence over ?:. So, while I still don't understand
why you think || would be equal to the current code, but expanding the paren throughout
the whole ?: expression seems to be required.

Note that the paren was intentionally placed like this under the assumption that ?:
has precedence over |. Which seems to be wrong.

> 
> > 
> > please review.
> > -------------------------->8------------------8<---------------------------
> > Fix misplaced parentheses
> > 
> > Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> > ---
> > diff --git a/drivers/net/wireless/b43/lo.c b/drivers/net/wireless/b43/lo.c
> > index 6a18a14..88ed75f 100644
> > --- a/drivers/net/wireless/b43/lo.c
> > +++ b/drivers/net/wireless/b43/lo.c
> > @@ -783,7 +783,7 @@ struct b43_lo_calib * b43_calibrate_lo_setting(struct b43_wldev *dev,
> >  			  | rfatt->att);
> >  	b43_radio_write16(dev, txctl_reg,
> >  			  (b43_radio_read16(dev, txctl_reg) & ~txctl_value)
> > -			  | (rfatt->with_padmix) ? txctl_value : 0);
> > +			  | (rfatt->with_padmix ? txctl_value : 0));
> >  
> >  	max_rx_gain = rfatt->att * 2;
> >  	max_rx_gain += bbatt->att / 2;
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> 
> 
> 



-- 
Greetings, Michael.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] B43: misplaced parentheses?
  2009-02-15 16:37 [PATCH] B43: misplaced parentheses? Roel Kluin
  2009-02-15 16:46 ` Michael Buesch
@ 2009-02-15 17:05 ` Larry Finger
  2009-02-15 17:07   ` Michael Buesch
  1 sibling, 1 reply; 6+ messages in thread
From: Larry Finger @ 2009-02-15 17:05 UTC (permalink / raw)
  To: Roel Kluin; +Cc: stefano.brivio, linux-wireless, Andrew Morton

Roel Kluin wrote:
> -			  | (rfatt->with_padmix) ? txctl_value : 0);
> +			  | (rfatt->with_padmix ? txctl_value : 0));

As I see it, the difference between the above two lines is the same as the
difference between c and d in this program:

int main(int argc, char **argv)
{
        int a = 1, b = 2, c, d;

        c = (a) ? b : 0;
        d = (a ? b : 0);
        fprintf (stderr, "a, b, c, d: %d %d %d %d\n", a, b, c, d);
        return 1;
}

When this program is executed, the output line is

a, b, c, d: 1 2 2 2

which is what I expected. Thus "(condition) ? result1 : result2" is the same as
"(condition ? result1 : result2)".

Larry

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] B43: misplaced parentheses?
  2009-02-15 17:05 ` Larry Finger
@ 2009-02-15 17:07   ` Michael Buesch
  2009-02-15 17:09     ` Larry Finger
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Buesch @ 2009-02-15 17:07 UTC (permalink / raw)
  To: Larry Finger; +Cc: Roel Kluin, stefano.brivio, linux-wireless, Andrew Morton

On Sunday 15 February 2009 18:05:52 Larry Finger wrote:
> Roel Kluin wrote:
> > -			  | (rfatt->with_padmix) ? txctl_value : 0);
> > +			  | (rfatt->with_padmix ? txctl_value : 0));
> 
> As I see it, the difference between the above two lines is the same as the
> difference between c and d in this program:
> 
> int main(int argc, char **argv)
> {
>         int a = 1, b = 2, c, d;
> 
>         c = (a) ? b : 0;
>         d = (a ? b : 0);
>         fprintf (stderr, "a, b, c, d: %d %d %d %d\n", a, b, c, d);
>         return 1;
> }
> 
> When this program is executed, the output line is
> 
> a, b, c, d: 1 2 2 2
> 
> which is what I expected. Thus "(condition) ? result1 : result2" is the same as
> "(condition ? result1 : result2)".

The problem is the binary OR operator, which has precedence over the ?: operator.


-- 
Greetings, Michael.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] B43: misplaced parentheses?
  2009-02-15 17:07   ` Michael Buesch
@ 2009-02-15 17:09     ` Larry Finger
  0 siblings, 0 replies; 6+ messages in thread
From: Larry Finger @ 2009-02-15 17:09 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Roel Kluin, stefano.brivio, linux-wireless, Andrew Morton

Michael Buesch wrote:
> On Sunday 15 February 2009 18:05:52 Larry Finger wrote:
>> Roel Kluin wrote:
>>> -			  | (rfatt->with_padmix) ? txctl_value : 0);
>>> +			  | (rfatt->with_padmix ? txctl_value : 0));
>> As I see it, the difference between the above two lines is the same as the
>> difference between c and d in this program:
>>
>> int main(int argc, char **argv)
>> {
>>         int a = 1, b = 2, c, d;
>>
>>         c = (a) ? b : 0;
>>         d = (a ? b : 0);
>>         fprintf (stderr, "a, b, c, d: %d %d %d %d\n", a, b, c, d);
>>         return 1;
>> }
>>
>> When this program is executed, the output line is
>>
>> a, b, c, d: 1 2 2 2
>>
>> which is what I expected. Thus "(condition) ? result1 : result2" is the same as
>> "(condition ? result1 : result2)".
> 
> The problem is the binary OR operator, which has precedence over the ?: operator.

I see. Please ignore my noise.

Larry

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-02-15 17:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-15 16:37 [PATCH] B43: misplaced parentheses? Roel Kluin
2009-02-15 16:46 ` Michael Buesch
2009-02-15 16:59   ` Michael Buesch
2009-02-15 17:05 ` Larry Finger
2009-02-15 17:07   ` Michael Buesch
2009-02-15 17:09     ` Larry Finger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).