Skip to contentSkip to conversation

B43: misplaced parentheses?

PATCH In review Roel Kluin 3 participants 6 messages Feb 15, 2009 Networking drivers (wireless)

lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 B43: misplaced parentheses?

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 <[email protected]>

---

drivers/net/wireless/b43/lo.c +1 -1
index 6a18a14..88ed75f 100644
@@ -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;
#1 lore raw
Quoted 7 lines · 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:

drivers/net/wireless/b43/lo.c +0 -0
 	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 ?:

Quoted 26 lines

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

Signed-off-by Roel Kluin <[email protected]>

---

drivers/net/wireless/b43/lo.c +1 -2
index 6a18a14..88ed75f 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 1 line
Greetings, Michael.
#2 lore raw
Quoted 11 lines · On Sunday 15 February 2009 17:46:44 Michael Buesch wrote:
Quoted 7 lines · 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:

drivers/net/wireless/b43/lo.c +0 -0
 	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.

Quoted 30 lines
Quoted 27 lines

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

Signed-off-by Roel Kluin <[email protected]>

---

drivers/net/wireless/b43/lo.c +1 -2
index 6a18a14..88ed75f 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 1 line
Greetings, Michael.
#3 lore raw
In reply to Roel Kluin

drivers/net/wireless/b43/lo.c struct b43_lo_calib * b43_calibrate_lo_setting(struct b43_wldev *dev, · line 786

Quoted 2 lines · Roel Kluin
drivers/net/wireless/b43/lo.c +1 -1
-			  | (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

#4 lore raw
Quoted 17 lines · On Sunday 15 February 2009 18:05:52 Larry Finger wrote:

drivers/net/wireless/b43/lo.c struct b43_lo_calib * b43_calibrate_lo_setting(struct b43_wldev *dev, · line 786

Quoted 2 lines · Roel Kluin
drivers/net/wireless/b43/lo.c +1 -1
-			  | (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;
}
Quoted 6 lines

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.

Signature · 1 line
Greetings, Michael.
#5 lore raw
Quoted 17 lines · Michael Buesch
Quoted 16 lines · On Sunday 15 February 2009 18:05:52 Larry Finger wrote:

drivers/net/wireless/b43/lo.c struct b43_lo_calib * b43_calibrate_lo_setting(struct b43_wldev *dev, · line 786

Quoted 2 lines · Roel Kluin
drivers/net/wireless/b43/lo.c +1 -1
-			  | (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;
}
Quoted 8 lines
Quoted 6 lines

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