From: Michael Straube <straube.linux@gmail.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: gregkh@linuxfoundation.org, Larry.Finger@lwfinger.net,
phil@philpotter.co.uk, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: r8188eu: remove unncessary ternary operator
Date: Mon, 4 Apr 2022 16:15:11 +0200 [thread overview]
Message-ID: <00574358-05fd-d80f-e052-ba9549a120ed@gmail.com> (raw)
In-Reply-To: <20220404130854.GB3293@kadam>
On 4/4/22 15:08, Dan Carpenter wrote:
> On Sat, Apr 02, 2022 at 07:38:35PM +0200, Michael Straube wrote:
>> diff --git a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
>> index 45b788212628..18650cd96f09 100644
>> --- a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
>> +++ b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
>> @@ -137,7 +137,7 @@ void rtl8188e_Add_RateATid(struct adapter *pAdapter, u32 bitmap, u8 arg, u8 rssi
>>
>> bitmap |= ((raid << 28) & 0xf0000000);
>>
>> - short_gi_rate = (arg & BIT(5)) ? true : false;
>> + short_gi_rate = arg & BIT(5);
>>
>
> This is a bug. Valid values of short_gi_rate are 0 and 1 but this sets
> it to BIT(5) so it will break odm_RateUp_8188E() which tests for:
>
> else if ((pRaInfo->SGIEnable) != 1)
>
Oh I see, I should have been more careful with this type of removals.
>> raid = (bitmap >> 28) & 0x0f;
>>
>> diff --git a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
>> index 6811be95da9a..ffc82d17ddbe 100644
>> --- a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
>> +++ b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
>> @@ -747,7 +747,7 @@ void Hal_ReadPowerSavingMode88E(struct adapter *padapter, u8 *hwinfo, bool AutoL
>>
>> /* decide hw if support remote wakeup function */
>> /* if hw supported, 8051 (SIE) will generate WeakUP signal(D+/D- toggle) when autoresume */
>> - padapter->pwrctrlpriv.bSupportRemoteWakeup = (hwinfo[EEPROM_USB_OPTIONAL_FUNCTION0] & BIT(1)) ? true : false;
>> + padapter->pwrctrlpriv.bSupportRemoteWakeup = hwinfo[EEPROM_USB_OPTIONAL_FUNCTION0] & BIT(1);
>
> I have not looked at this one to see if has a similar issue... I don't
> necessarily want to audit all of these. I guess I will, but could you
> do it first and then I will check?
>
Sure I'll do.
Thanks,
Michael
>> diff --git a/drivers/staging/r8188eu/include/ieee80211.h b/drivers/staging/r8188eu/include/ieee80211.h
>> index 8c20363cdd31..03d63257797a 100644
>> --- a/drivers/staging/r8188eu/include/ieee80211.h
>> +++ b/drivers/staging/r8188eu/include/ieee80211.h
>> @@ -127,7 +127,7 @@ enum NETWORK_TYPE {
>> (WIRELESS_11B | WIRELESS_11G | WIRELESS_11_24N)
>>
>> #define IsSupported24G(NetType) \
>> - ((NetType) & SUPPORTED_24G_NETTYPE_MSK ? true : false)
>> + ((NetType) & SUPPORTED_24G_NETTYPE_MSK)
>
> This too.
>
>>
>> #define IsEnableHWCCK(NetType) \
>> IsSupported24G(NetType)
>> @@ -135,11 +135,11 @@ enum NETWORK_TYPE {
>> #define IsSupportedRxCCK(NetType) IsEnableHWCCK(NetType)
>>
>> #define IsSupportedTxCCK(NetType) \
>> - ((NetType) & (WIRELESS_11B) ? true : false)
>> + ((NetType) & WIRELESS_11B)
>> #define IsSupportedTxOFDM(NetType) \
>> - ((NetType) & (WIRELESS_11G) ? true : false)
>> + ((NetType) & WIRELESS_11G)
>> #define IsSupportedTxMCS(NetType) \
>> - ((NetType) & (WIRELESS_11_24N) ? true : false)
>> + ((NetType) & WIRELESS_11_24N)
>>
>
> And this.
>
> regards,
> dan carpenter
>
next prev parent reply other threads:[~2022-04-04 14:15 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-02 17:38 [PATCH] staging: r8188eu: remove unncessary ternary operator Michael Straube
2022-04-04 13:08 ` Dan Carpenter
2022-04-04 14:15 ` Michael Straube [this message]
2022-04-04 17:00 ` Michael Straube
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=00574358-05fd-d80f-e052-ba9549a120ed@gmail.com \
--to=straube.linux@gmail.com \
--cc=Larry.Finger@lwfinger.net \
--cc=dan.carpenter@oracle.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=phil@philpotter.co.uk \
/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.