b43: Fix bogus compilation warning for phy_n

PATCH New Larry Finger 2 participants 7 messages May 19, 2011 Networking drivers (wireless)

Share & tools

Larry Finger

author

2011-05-19 21:35 UTC #0 lore raw

PATCH 1/1 b43: Fix bogus compilation warning for phy_n

When cross-compiling the 2.6.39 wireless-testing source using GCC version (SUSE Linux) 4.3.2 [gcc-4_3-branch revision 141291] on an x86_64 system, the following warning is issued:

CC [M] drivers/net/wireless/b43/phy_n.o
drivers/net/wireless/b43/phy_n.c: In function ?b43_nphy_cal_tx_iq_lo?:
drivers/net/wireless/b43/phy_n.c:3096: warning: ?last? may be used
uninitialized in this function

A quick look at the code shows that the warning is bogus and a gcc bug, but to ensure clean compilation for all users, mark the offending variable as uninitialized.

Signed-off-by Larry Finger <[email protected]>

---

drivers/net/wireless/b43/phy_n.c +1 -1

Index: wireless-testing-new/drivers/net/wireless/b43/phy_n.c
===================================================================
@@ -3093,7 +3093,7 @@ static int b43_nphy_cal_tx_iq_lo(struct
 	int freq;
 	bool avoid = false;
 	u8 length;
-	u16 tmp, core, type, count, max, numb, last, cmd;
+	u16 tmp, core, type, count, max, numb, uninitialized_var(last), cmd;
 	const u16 *table;
 	bool phy6or5x;

Rafał Miłecki

2011-05-19 21:43 UTC #1 lore raw

PATCH 1/1 b43: Fix bogus compilation warning for phy_n

2011/5/19 Larry Finger <[email protected]>:

Quoted 4 lines

When cross-compiling the 2.6.39 wireless-testing source using GCC version (SUSE Linux) 4.3.2 [gcc-4_3-branch revision 141291] on an x86_64 system, the following warning is issued:

Quoted 8 lines

?CC [M] ?drivers/net/wireless/b43/phy_n.o drivers/net/wireless/b43/phy_n.c: In function ?b43_nphy_cal_tx_iq_lo?:
drivers/net/wireless/b43/phy_n.c:3096: warning: ?last? may be used ? ? ? ?uninitialized in this function
A quick look at the code shows that the warning is bogus and a gcc bug, but to ensure clean compilation for all users, mark the offending variable as uninitialized.

Did you check for both "last" usages on this function? From my quick review it seems "last" is set in case of
1) mphase_cal_phase_id > 2
xor
2) b43_nphy_tx_tone returning success

I'm not so sure if this patch is correct.

Signature · 1 line

Rafa?

Larry Finger

author

2011-05-19 22:12 UTC #2 lore raw

PATCH 1/1 b43: Fix bogus compilation warning for phy_n

Quoted 14 lines · Rafa? Mi?ecki

2011/5/19 Larry Finger<[email protected]>:
Quoted 4 lines
When cross-compiling the 2.6.39 wireless-testing source using GCC version (SUSE Linux) 4.3.2 [gcc-4_3-branch revision 141291] on an x86_64 system, the following warning is issued:
Quoted 8 lines
CC [M] drivers/net/wireless/b43/phy_n.o
drivers/net/wireless/b43/phy_n.c: In function ?b43_nphy_cal_tx_iq_lo?:
drivers/net/wireless/b43/phy_n.c:3096: warning: ?last? may be used
uninitialized in this function
A quick look at the code shows that the warning is bogus and a gcc bug, but to ensure clean compilation for all users, mark the offending variable as uninitialized.

Quoted 7 lines

Did you check for both "last" usages on this function? From my quick review it seems "last" is set in case of
1) mphase_cal_phase_id> 2
xor
2) b43_nphy_tx_tone returning success
I'm not so sure if this patch is correct.

My analysis is as follows: "last" is created in line 3096. In line 3256, it is set by the statement "last = (dev->phy.rev < 3) ? 6 : 7;". In line 3258 and 3300, it is tested for equality with "nphy->mphase_cal_phase_id". As there is no path around line 3256, it seems to me that last must be assigned a value at 3256 and the warning is bogus.

The call in line 3154 to b43_nphy_tx_tone is "error = b43_nphy_tx_tone(dev, freq, 250, true, false);" and does not access last.

If this patch is not correct, then last must be initialized to zero and the older compiler is correct and the newer ones are buggy for not reporting the problem.

Larry

Rafał Miłecki

2011-05-19 22:40 UTC #3 lore raw

PATCH 1/1 b43: Fix bogus compilation warning for phy_n

W dniu 20 maja 2011 00:12 u?ytkownik Larry Finger
<[email protected]> napisa?:

Quoted 32 lines · Rafa? Mi?ecki

Quoted 17 lines
2011/5/19 Larry Finger<[email protected]>:
Quoted 10 lines
When cross-compiling the 2.6.39 wireless-testing source using GCC version (SUSE Linux) 4.3.2 [gcc-4_3-branch revision 141291] on an x86_64 system, the following warning is issued:
?CC [M] ?drivers/net/wireless/b43/phy_n.o drivers/net/wireless/b43/phy_n.c: In function ?b43_nphy_cal_tx_iq_lo?:
drivers/net/wireless/b43/phy_n.c:3096: warning: ?last? may be used ? ? ? ?uninitialized in this function
Quoted 4 lines
A quick look at the code shows that the warning is bogus and a gcc bug, but to ensure clean compilation for all users, mark the offending variable
as uninitialized.
Quoted 7 lines
Did you check for both "last" usages on this function? From my quick review it seems "last" is set in case of
1) mphase_cal_phase_id> ?2
xor
2) b43_nphy_tx_tone returning success
I'm not so sure if this patch is correct.
My analysis is as follows: "last" is created in line 3096. In line 3256, it is set by the statement "last = (dev->phy.rev < 3) ? 6 : 7;". In line 3258 and 3300, it is tested for equality with "nphy->mphase_cal_phase_id". As there is no path around line 3256, it seems to me that last must be assigned a value at 3256 and the warning is bogus.

Quoted 6 lines

The call in line 3154 to b43_nphy_tx_tone is "error = b43_nphy_tx_tone(dev, freq, 250, true, false);" and does not access last.
If this patch is not correct, then last must be initialized to zero and the older compiler is correct and the newer ones are buggy for not reporting the problem.

I should have describe where I can see the problem.

If error is other than 0 (it can be as the result of "error = b43_nphy_tx_tone(dev, freq, 250, true, false);"), then "last" won't be set in 3256. In 3300 we use "last", no matter what "error" is.

Signature · 1 line

Rafa?

Larry Finger

author

2011-05-19 22:48 UTC #4 lore raw

PATCH 1/1 b43: Fix bogus compilation warning for phy_n

Quoted 6 lines · Rafa? Mi?ecki

I should have describe where I can see the problem.
If error is other than 0 (it can be as the result of "error = b43_nphy_tx_tone(dev, freq, 250, true, false);"), then "last" won't be set in 3256. In 3300 we use "last", no matter what "error" is.

You are correct. I missed the level of indent in line 3256. What initialization should be used for last? It clearly needs to be set to something.

John: Please drop my patch. I'll let Rafa? fix it.

Larry

Rafał Miłecki

2011-05-19 22:59 UTC #5 lore raw

PATCH 1/1 b43: Fix bogus compilation warning for phy_n

W dniu 20 maja 2011 00:48 u?ytkownik Larry Finger
<[email protected]> napisa?:

Quoted 11 lines · Rafa? Mi?ecki

Quoted 6 lines
I should have describe where I can see the problem.
If error is other than 0 (it can be as the result of "error = b43_nphy_tx_tone(dev, freq, 250, true, false);"), then "last" won't be set in 3256. In 3300 we use "last", no matter what "error" is.
You are correct. I missed the level of indent in line 3256. What initialization should be used for last? ?It clearly needs to be set to something.

http://bcm-v4.sipsolutions.net/802.11/PHY/N/CalTxIqlo does not say anything about initial value. Guess it should be zero.

Signature · 1 line

Rafa?

Larry Finger

author

2011-05-19 23:07 UTC #6 lore raw

PATCH 1/1 b43: Fix bogus compilation warning for phy_n

Quoted 7 lines · Rafa? Mi?ecki

W dniu 20 maja 2011 00:48 u?ytkownik Larry Finger
Quoted 3 lines
You are correct. I missed the level of indent in line 3256. What initialization should be used for last? It clearly needs to be set to something.
http://bcm-v4.sipsolutions.net/802.11/PHY/N/CalTxIqlo does not say anything about initial value. Guess it should be zero.

I checked the reference driver. It should be zero. The specs are updated.

Larry