From: Pkshih <pkshih@realtek.com>
To: "mka@chromium.org" <mka@chromium.org>,
"Larry.Finger@lwfinger.net" <Larry.Finger@lwfinger.net>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
"kvalo@codeaurora.org" <kvalo@codeaurora.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"craigb@chromium.org" <craigb@chromium.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"groeck@chromium.org" <groeck@chromium.org>,
"teravest@chromium.org" <teravest@chromium.org>
Subject: Re: [PATCH] rtlwifi: rtl8192cu: Remove variable self-assignment in rf.c
Date: Thu, 8 Feb 2018 01:06:43 +0000 [thread overview]
Message-ID: <1518052003.2680.1.camel@realtek.com> (raw)
In-Reply-To: <20180207205136.GB116483@google.com>
T24gV2VkLCAyMDE4LTAyLTA3IGF0IDEyOjUxIC0wODAwLCBNYXR0aGlhcyBLYWVobGNrZSB3cm90
ZToNCj4gRWwgV2VkLCBGZWIgMDcsIDIwMTggYXQgMDI6MzU6NTlQTSAtMDYwMCBMYXJyeSBGaW5n
ZXIgaGEgZGl0Og0KPiANCj4gPiBPbiAwMi8wNy8yMDE4IDAyOjI2IFBNLCBNYXR0aGlhcyBLYWVo
bGNrZSB3cm90ZToNCj4gPiA+IEluIF9ydGw5MmNfZ2V0X3R4cG93ZXJfd3JpdGV2YWxfYnlfcmVn
dWxhdG9yeSgpIHRoZSB2YXJpYWJsZSB3cml0ZVZhbA0KPiA+ID4gaXMgYXNzaWduZWQgdG8gaXRz
ZWxmIGluIGFuIGlmIC4uLiBlbHNlIHN0YXRlbWVudCwgYXBwYXJlbnRseSBvbmx5IHRvDQo+ID4g
PiBkb2N1bWVudCB0aGF0IHRoZSBicmFuY2ggY29uZGl0aW9uIGlzIGhhbmRsZWQgYW5kIHRoYXQg
YSBwcmV2aW91c2x5IHJlYWQNCj4gPiA+IHZhbHVlIHNob3VsZCBiZSByZXR1cm5lZCB1bm1vZGlm
aWVkLiBUaGUgc2VsZi1hc3NpZ25tZW50IGNhdXNlcyBjbGFuZyB0bw0KPiA+ID4gcmFpc2UgdGhl
IGZvbGxvd2luZyB3YXJuaW5nOg0KPiA+ID7CoA0KPiA+ID4gZHJpdmVycy9uZXQvd2lyZWxlc3Mv
cmVhbHRlay9ydGx3aWZpL3J0bDgxOTJjdS9yZi5jOjMwNDoxMzoNCj4gPiA+wqDCoMKgwqBlcnJv
cjogZXhwbGljaXRseSBhc3NpZ25pbmcgdmFsdWUgb2YgdmFyaWFibGUgb2YgdHlwZSAndTMyJw0K
PiA+ID7CoMKgwqDCoMKgwqAoYWthICd1bnNpZ25lZCBpbnQnKSB0byBpdHNlbGYgWy1XZXJyb3Is
LVdzZWxmLWFzc2lnbl0NCj4gPiA+wqDCoMKgwqB3cml0ZVZhbCA9IHdyaXRlVmFsOw0KPiA+ID7C
oA0KPiA+ID4gUmVwbGFjZSB0aGUgc2VsZi1hc3NpZ25tZW50IHdpdGggYSBzZW1pY29sb24sIHdo
aWNoIHN0aWxsIHNlcnZlcyB0bw0KPiA+ID4gZG9jdW1lbnQgdGhlICdoYW5kbGluZycgb2YgdGhl
IGJyYW5jaCBjb25kaXRpb24uDQo+ID4gPsKgDQo+ID4gPiBTaWduZWQtb2ZmLWJ5OiBNYXR0aGlh
cyBLYWVobGNrZSA8bWthQGNocm9taXVtLm9yZz4NCj4gPiA+IC0tLQ0KPiA+ID7CoMKgwqBkcml2
ZXJzL25ldC93aXJlbGVzcy9yZWFsdGVrL3J0bHdpZmkvcnRsODE5MmN1L3JmLmMgfCAyICstDQo+
ID4gPsKgwqDCoDEgZmlsZSBjaGFuZ2VkLCAxIGluc2VydGlvbigrKSwgMSBkZWxldGlvbigtKQ0K
PiA+ID7CoA0KPiA+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL3JlYWx0ZWsv
cnRsd2lmaS9ydGw4MTkyY3UvcmYuYw0KPiBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL3JlYWx0ZWsv
cnRsd2lmaS9ydGw4MTkyY3UvcmYuYw0KPiA+ID4gaW5kZXggOWNmZjZiYzQwNDljLi40ZGI5MjQ5
NmMxMjIgMTAwNjQ0DQo+ID4gPiAtLS0gYS9kcml2ZXJzL25ldC93aXJlbGVzcy9yZWFsdGVrL3J0
bHdpZmkvcnRsODE5MmN1L3JmLmMNCj4gPiA+ICsrKyBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL3Jl
YWx0ZWsvcnRsd2lmaS9ydGw4MTkyY3UvcmYuYw0KPiA+ID4gQEAgLTMwMSw3ICszMDEsNyBAQCBz
dGF0aWMgdm9pZCBfcnRsOTJjX2dldF90eHBvd2VyX3dyaXRldmFsX2J5X3JlZ3VsYXRvcnkoc3Ry
dWN0IGllZWU4MDIxMV9odw0KPiAqaHcsDQo+ID4gPsKgwqDCoAkJCXdyaXRlVmFsID0gd3JpdGVW
YWwgLSAweDA2MDYwNjA2Ow0KPiA+ID7CoMKgwqAJCWVsc2UgaWYgKHJ0bHByaXYtPmRtLmR5bmFt
aWNfdHhoaWdocG93ZXJfbHZsID09DQo+ID4gPsKgwqDCoAkJCcKgVFhISUdIUFdSTEVWRUxfQlQy
KQ0KPiA+ID4gLQkJCXdyaXRlVmFsID0gd3JpdGVWYWw7DQo+ID4gPiArCQkJOw0KPiA+ID7CoMKg
wqAJCSoocF9vdXR3cml0ZXZhbCArIHJmKSA9IHdyaXRlVmFsOw0KPiA+ID7CoMKgwqAJfQ0KPiA+
ID7CoMKgwqB9DQo+ID4gPsKgDQo+ID7CoA0KPiA+IEFzIHRoZSBicmFuY2ggY29uZGl0aW9uIGRv
ZXMgbm90aGluZywgd2h5IG5vdCByZW1vdmUgaXQgYW5kIHNhdmUgdGhlDQo+ID4gY29tcGlsZXIn
cyBvcHRpbWl6ZXIgYSBiaXQgb2Ygd29yaz8gVGhlIGNvZGUgbG9va3Mgc3RyYW5nZSwgYnV0IGl0
IG1hdGNoZXMNCj4gPiB0aGUgcmVzdCBvZiBSZWFsdGVrJ3MgVVNCIGRyaXZlcnMuDQoNCkFncmVl
IExhcnJ5J3MgY29tbWVudC4NCg0KPiANCj4gU3VyZSwgSSBhbSBoYXBweSB0byBjaGFuZ2UgaXQg
dG8gd2hhdGV2ZXIgdGhlIGF1dGhvcnMvbWFpbnRhaW5lcnMgcHJlZmVyLg0KPiANCj4gSSdsbCB3
YWl0IGEgYml0IGJlZm9yZSByZXNwaW5uaW5nIGZvciBpZiBvdGhlcnMgZmVlbCBzdHJvbmdseSBh
Ym91dA0KPiBrZWVwaW5nIHRoZSBicmFuY2guDQo+IA0KPiAtLS0tLS1QbGVhc2UgY29uc2lkZXIg
dGhlIGVudmlyb25tZW50IGJlZm9yZSBwcmludGluZyB0aGlzIGUtbWFpbC4=
WARNING: multiple messages have this Message-ID (diff)
From: Pkshih <pkshih@realtek.com>
To: "mka@chromium.org" <mka@chromium.org>,
"Larry.Finger@lwfinger.net" <Larry.Finger@lwfinger.net>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
"kvalo@codeaurora.org" <kvalo@codeaurora.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"craigb@chromium.org" <craigb@chromium.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"groeck@chromium.org" <groeck@chromium.org>,
"teravest@chromium.org" <teravest@chromium.org>
Subject: Re: [PATCH] rtlwifi: rtl8192cu: Remove variable self-assignment in rf.c
Date: Thu, 8 Feb 2018 01:06:43 +0000 [thread overview]
Message-ID: <1518052003.2680.1.camel@realtek.com> (raw)
In-Reply-To: <20180207205136.GB116483@google.com>
On Wed, 2018-02-07 at 12:51 -0800, Matthias Kaehlcke wrote:
> El Wed, Feb 07, 2018 at 02:35:59PM -0600 Larry Finger ha dit:
>
> > On 02/07/2018 02:26 PM, Matthias Kaehlcke wrote:
> > > In _rtl92c_get_txpower_writeval_by_regulatory() the variable writeVal
> > > is assigned to itself in an if ... else statement, apparently only to
> > > document that the branch condition is handled and that a previously read
> > > value should be returned unmodified. The self-assignment causes clang to
> > > raise the following warning:
> > >
> > > drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c:304:13:
> > > error: explicitly assigning value of variable of type 'u32'
> > > (aka 'unsigned int') to itself [-Werror,-Wself-assign]
> > > writeVal = writeVal;
> > >
> > > Replace the self-assignment with a semicolon, which still serves to
> > > document the 'handling' of the branch condition.
> > >
> > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > > ---
> > > drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c
> b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c
> > > index 9cff6bc4049c..4db92496c122 100644
> > > --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c
> > > +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c
> > > @@ -301,7 +301,7 @@ static void _rtl92c_get_txpower_writeval_by_regulatory(struct ieee80211_hw
> *hw,
> > > writeVal = writeVal - 0x06060606;
> > > else if (rtlpriv->dm.dynamic_txhighpower_lvl ==
> > > TXHIGHPWRLEVEL_BT2)
> > > - writeVal = writeVal;
> > > + ;
> > > *(p_outwriteval + rf) = writeVal;
> > > }
> > > }
> > >
> >
> > As the branch condition does nothing, why not remove it and save the
> > compiler's optimizer a bit of work? The code looks strange, but it matches
> > the rest of Realtek's USB drivers.
Agree Larry's comment.
>
> Sure, I am happy to change it to whatever the authors/maintainers prefer.
>
> I'll wait a bit before respinning for if others feel strongly about
> keeping the branch.
>
> ------Please consider the environment before printing this e-mail.
next prev parent reply other threads:[~2018-02-08 1:07 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-07 20:26 [PATCH] rtlwifi: rtl8192cu: Remove variable self-assignment in rf.c Matthias Kaehlcke
2018-02-07 20:35 ` Larry Finger
2018-02-07 20:51 ` Matthias Kaehlcke
2018-02-08 1:06 ` Pkshih [this message]
2018-02-08 1:06 ` Pkshih
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=1518052003.2680.1.camel@realtek.com \
--to=pkshih@realtek.com \
--cc=Larry.Finger@lwfinger.net \
--cc=craigb@chromium.org \
--cc=groeck@chromium.org \
--cc=kvalo@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=mka@chromium.org \
--cc=netdev@vger.kernel.org \
--cc=teravest@chromium.org \
/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.