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]>
---
Index: wireless-testing-new/drivers/net/wireless/b43/phy_n.c
===================================================================
--- wireless-testing-new.orig/drivers/net/wireless/b43/phy_n.c
+++ 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;
W dniu 20 maja 2011 00:48 użytkownik Larry Finger
<[email protected]> napisał:
> On 05/19/2011 05:40 PM, Rafał Miłecki wrote:
>>
>> 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.
--
Rafał
On 05/19/2011 04:43 PM, Rafał Miłecki wrote:
> 2011/5/19 Larry Finger<[email protected]>:
>> 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.
>
> 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
W dniu 20 maja 2011 00:12 użytkownik Larry Finger
<[email protected]> napisał:
> On 05/19/2011 04:43 PM, Rafał Miłecki wrote:
>>
>> 2011/5/19 Larry Finger<[email protected]>:
>>>
>>> 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.
>>
>> 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.
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.
--
Rafał
On 05/19/2011 05:40 PM, Rafał Miłecki wrote:
>
> 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
2011/5/19 Larry Finger <[email protected]>:
> 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.
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.
--
Rafał
On 05/19/2011 05:59 PM, Rafał Miłecki wrote:
> W dniu 20 maja 2011 00:48 użytkownik Larry Finger
>> 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