From: Nathan Chancellor <natechancellor@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: rtl8188eu: Simplify memcmp logical checks
Date: Tue, 25 Sep 2018 12:15:04 -0700 [thread overview]
Message-ID: <20180925191504.GA4454@flashbox> (raw)
In-Reply-To: <20180925190711.GA31631@kroah.com>
On Tue, Sep 25, 2018 at 09:07:11PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Sep 20, 2018 at 05:22:21PM -0700, Nathan Chancellor wrote:
> > Clang generates a warning when it sees a logical not followed by a
> > conditional operator like ==, >, or < because it thinks that the logical
> > not should be applied to the whole statement:
> >
> > drivers/staging/rtl8188eu/core/rtw_ieee80211.c:293:8: warning: logical
> > not is only applied to the left hand side of this comparison
> > [-Wlogical-not-parentheses]
> >
> > It assumes the author might have made a mistake in their logic:
> >
> > if (!a == b) -> if (!(a == b))
> >
> > Sometimes that is the case; other times, it's just a super convoluted
> > way of saying 'if (a)' when b = 0:
> >
> > if (!1 == 0) -> if (0 == 0) -> if (true)
> >
> > Alternatively:
> >
> > if (!1 == 0) -> if (!!1) -> if (1)
> >
> > Simplify these comparisons so that Clang doesn't complain.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/161
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> > drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 2 +-
> > drivers/staging/rtl8188eu/core/rtw_mlme.c | 2 +-
> > drivers/staging/rtl8188eu/core/rtw_recv.c | 4 ++--
> > drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 2 +-
> > 4 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
> > index 9e5c7e62d26f..20f34d25c369 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
> > @@ -284,7 +284,7 @@ unsigned char *rtw_get_wpa_ie(unsigned char *pie, uint *wpa_ie_len, int limit)
> >
> > if (pbuf) {
> > /* check if oui matches... */
> > - if (!memcmp((pbuf + 2), wpa_oui_type, sizeof(wpa_oui_type)) == false)
> > + if (memcmp((pbuf + 2), wpa_oui_type, sizeof(wpa_oui_type)))
> > goto check_next_ie;
> >
> > /* check version... */
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > index ef8a7dc4bd34..43d6513484c5 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > @@ -1437,7 +1437,7 @@ static int rtw_check_join_candidate(struct mlme_priv *pmlmepriv
> > /* check ssid, if needed */
> > if (pmlmepriv->assoc_ssid.SsidLength) {
> > if (competitor->network.Ssid.SsidLength != pmlmepriv->assoc_ssid.SsidLength ||
> > - !memcmp(competitor->network.Ssid.Ssid, pmlmepriv->assoc_ssid.Ssid, pmlmepriv->assoc_ssid.SsidLength) == false)
> > + memcmp(competitor->network.Ssid.Ssid, pmlmepriv->assoc_ssid.Ssid, pmlmepriv->assoc_ssid.SsidLength))
> > goto exit;
> > }
> >
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
> > index ab9638d618a9..f3eb63f8cf0b 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_recv.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
> > @@ -1283,8 +1283,8 @@ static int wlanhdr_to_ethhdr(struct recv_frame *precvframe)
> > psnap_type = ptr+pattrib->hdrlen + pattrib->iv_len+SNAP_SIZE;
> > /* convert hdr + possible LLC headers into Ethernet header */
> > if ((!memcmp(psnap, rtw_rfc1042_header, SNAP_SIZE) &&
> > - (!memcmp(psnap_type, SNAP_ETH_TYPE_IPX, 2) == false) &&
> > - (!memcmp(psnap_type, SNAP_ETH_TYPE_APPLETALK_AARP, 2) == false)) ||
> > + memcmp(psnap_type, SNAP_ETH_TYPE_IPX, 2) &&
> > + memcmp(psnap_type, SNAP_ETH_TYPE_APPLETALK_AARP, 2)) ||
> > !memcmp(psnap, rtw_bridge_tunnel_header, SNAP_SIZE)) {
> > /* remove RFC1042 or Bridge-Tunnel encapsulation and replace EtherType */
> > bsnaphdr = true;
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
> > index fb496ab5a862..51cf78150168 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
> > @@ -856,7 +856,7 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8 *pframe, u32 packet_len)
> > return _FAIL;
> > }
> >
> > - if (!memcmp(cur_network->network.MacAddress, pbssid, 6) == false) {
> > + if (memcmp(cur_network->network.MacAddress, pbssid, 6) {
> > DBG_88E("Oops: rtw_check_network_encrypt linked but recv other bssid bcn\n%pM %pM\n",
> > (pbssid), (cur_network->network.MacAddress));
> > return true;
>
> Always test-build your patches before sending them off so you do not get
> a grumpy maintainer upset at you for breaking their build system...
>
> Hint, it's the last chunk here...
>
> greg k-h
Gah sorry, it's always the times that I don't do that that something bad
happens :(
v2 sent.
Nathan
next prev parent reply other threads:[~2018-09-25 19:15 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-21 0:22 [PATCH] staging: rtl8188eu: Simplify memcmp logical checks Nathan Chancellor
2018-09-25 19:07 ` Greg Kroah-Hartman
2018-09-25 19:15 ` Nathan Chancellor [this message]
2018-09-25 19:13 ` [PATCH v2] " Nathan Chancellor
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=20180925191504.GA4454@flashbox \
--to=natechancellor@gmail.com \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.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 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).