From: Larry Finger <Larry.Finger@lwfinger.net>
To: "Rafał Miłecki" <zajec5@gmail.com>
Cc: John W Linville <linville@tuxdriver.com>,
b43-dev@lists.infradead.org, linux-wireless@vger.kernel.org
Subject: [PATCH] b43: Fix bogus compilation warning for phy_n
Date: Thu, 19 May 2011 17:12:25 -0500 [thread overview]
Message-ID: <4DD595C9.105@lwfinger.net> (raw)
In-Reply-To: <BANLkTinDciqW2yJLpwKemnSM=8_1Jy3Fxw@mail.gmail.com>
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
WARNING: multiple messages have this Message-ID (diff)
From: Larry Finger <Larry.Finger@lwfinger.net>
To: "Rafał Miłecki" <zajec5@gmail.com>
Cc: John W Linville <linville@tuxdriver.com>,
b43-dev@lists.infradead.org, linux-wireless@vger.kernel.org
Subject: Re: [PATCH] b43: Fix bogus compilation warning for phy_n
Date: Thu, 19 May 2011 17:12:25 -0500 [thread overview]
Message-ID: <4DD595C9.105@lwfinger.net> (raw)
In-Reply-To: <BANLkTinDciqW2yJLpwKemnSM=8_1Jy3Fxw@mail.gmail.com>
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
next prev parent reply other threads:[~2011-05-19 22:12 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-19 21:35 [PATCH] b43: Fix bogus compilation warning for phy_n Larry Finger
2011-05-19 21:35 ` Larry Finger
2011-05-19 21:43 ` Rafał Miłecki
2011-05-19 21:43 ` Rafał Miłecki
2011-05-19 22:12 ` Larry Finger [this message]
2011-05-19 22:12 ` Larry Finger
2011-05-19 22:40 ` Rafał Miłecki
2011-05-19 22:40 ` Rafał Miłecki
2011-05-19 22:48 ` Larry Finger
2011-05-19 22:48 ` Larry Finger
2011-05-19 22:59 ` Rafał Miłecki
2011-05-19 22:59 ` Rafał Miłecki
2011-05-19 23:07 ` Larry Finger
2011-05-19 23:07 ` Larry Finger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DD595C9.105@lwfinger.net \
--to=larry.finger@lwfinger.net \
--cc=b43-dev@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=zajec5@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.