* [PATCH] b43: Fix bogus compilation warning for phy_n
@ 2011-05-19 21:35 Larry Finger
2011-05-19 21:43 ` Rafał Miłecki
0 siblings, 1 reply; 7+ messages in thread
From: Larry Finger @ 2011-05-19 21:35 UTC (permalink / raw)
To: zajec5, John W Linville; +Cc: b43-dev, linux-wireless
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 <Larry.Finger@lwfinger.net>
---
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;
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] b43: Fix bogus compilation warning for phy_n
2011-05-19 21:35 [PATCH] b43: Fix bogus compilation warning for phy_n Larry Finger
@ 2011-05-19 21:43 ` Rafał Miłecki
2011-05-19 22:12 ` Larry Finger
0 siblings, 1 reply; 7+ messages in thread
From: Rafał Miłecki @ 2011-05-19 21:43 UTC (permalink / raw)
To: Larry Finger; +Cc: John W Linville, b43-dev, linux-wireless
2011/5/19 Larry Finger <Larry.Finger@lwfinger.net>:
> 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?
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] b43: Fix bogus compilation warning for phy_n
2011-05-19 21:43 ` Rafał Miłecki
@ 2011-05-19 22:12 ` Larry Finger
2011-05-19 22:40 ` Rafał Miłecki
0 siblings, 1 reply; 7+ messages in thread
From: Larry Finger @ 2011-05-19 22:12 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: John W Linville, b43-dev, linux-wireless
On 05/19/2011 04:43 PM, Rafa? Mi?ecki wrote:
> 2011/5/19 Larry Finger<Larry.Finger@lwfinger.net>:
>> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] b43: Fix bogus compilation warning for phy_n
2011-05-19 22:12 ` Larry Finger
@ 2011-05-19 22:40 ` Rafał Miłecki
2011-05-19 22:48 ` Larry Finger
0 siblings, 1 reply; 7+ messages in thread
From: Rafał Miłecki @ 2011-05-19 22:40 UTC (permalink / raw)
To: Larry Finger; +Cc: John W Linville, b43-dev, linux-wireless
W dniu 20 maja 2011 00:12 u?ytkownik Larry Finger
<Larry.Finger@lwfinger.net> napisa?:
> On 05/19/2011 04:43 PM, Rafa? Mi?ecki wrote:
>>
>> 2011/5/19 Larry Finger<Larry.Finger@lwfinger.net>:
>>>
>>> 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?
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] b43: Fix bogus compilation warning for phy_n
2011-05-19 22:40 ` Rafał Miłecki
@ 2011-05-19 22:48 ` Larry Finger
2011-05-19 22:59 ` Rafał Miłecki
0 siblings, 1 reply; 7+ messages in thread
From: Larry Finger @ 2011-05-19 22:48 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: John W Linville, b43-dev, linux-wireless
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] b43: Fix bogus compilation warning for phy_n
2011-05-19 22:48 ` Larry Finger
@ 2011-05-19 22:59 ` Rafał Miłecki
2011-05-19 23:07 ` Larry Finger
0 siblings, 1 reply; 7+ messages in thread
From: Rafał Miłecki @ 2011-05-19 22:59 UTC (permalink / raw)
To: Larry Finger; +Cc: John W Linville, b43-dev, linux-wireless
W dniu 20 maja 2011 00:48 u?ytkownik Larry Finger
<Larry.Finger@lwfinger.net> 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?
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-05-19 23:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-19 21:35 [PATCH] b43: Fix bogus compilation warning for phy_n Larry Finger
2011-05-19 21:43 ` Rafał Miłecki
2011-05-19 22:12 ` Larry Finger
2011-05-19 22:40 ` Rafał Miłecki
2011-05-19 22:48 ` Larry Finger
2011-05-19 22:59 ` Rafał Miłecki
2011-05-19 23:07 ` 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).