From: Luis de Bethencourt <luisbg@osg.samsung.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
florian.c.schilhabel@googlemail.com, punitvara@gmail.com,
stillcompiling@gmail.com, luca@lucaceresoli.net,
carlos@cgarcia.org, Julia.Lawall@lip6.fr, vthakkar1994@gmail.com,
gregkh@linuxfoundation.org, cristina.opriceana@gmail.com,
fabf@skynet.be, shivanib134@gmail.com, jteki@openedev.com,
sudipm.mukherjee@gmail.com, Larry.Finger@lwfinger.net
Subject: Re: [PATCH v2 1/3] staging: rtl8712: Remove boolean comparisons
Date: Sat, 24 Oct 2015 12:56:49 +0100 [thread overview]
Message-ID: <562B7201.6060604@osg.samsung.com> (raw)
In-Reply-To: <20151022190524.GP7340@mwanda>
On 22/10/15 20:05, Dan Carpenter wrote:
> On Mon, Oct 19, 2015 at 06:14:29PM +0100, Luis de Bethencourt wrote:
>> Boolean tests do not need explicit comparison to true or false.
>>
>> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
>> ---
>> diff --git a/drivers/staging/rtl8712/usb_ops_linux.c b/drivers/staging/rtl8712/usb_ops_linux.c
>> index c940722..e33eeed 100644
>> --- a/drivers/staging/rtl8712/usb_ops_linux.c
>> +++ b/drivers/staging/rtl8712/usb_ops_linux.c
>> @@ -266,7 +266,7 @@ u32 r8712_usb_read_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem)
>> if (adapter->bDriverStopped || adapter->bSurpriseRemoved ||
>> adapter->pwrctrlpriv.pnp_bstop_trx)
>> return _FAIL;
>> - if (!precvbuf->reuse == false || !precvbuf->pskb) {
>> + if (precvbuf->reuse || !precvbuf->pskb) {
>> precvbuf->pskb = skb_dequeue(&precvpriv->free_recv_skb_queue);
>> if (precvbuf->pskb != NULL)
>> precvbuf->reuse = true;
>
> You have transformed this faithfully, but my instinct says that the
> original code is wrong. It should be:
>
> if (!precvbuf->reuse || !precvbuf->pskb) {
>
> I checked and usb_read_port() is implemented this way in
> drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c. Again I am going on
> instinct and not a full understanding of the code, but I'm probably
> correct.
>
> Anyway, this is not related to the patch so we should fix it in a later
> patch, but let's not forget.
>
> TODO: rtl8712: fix a reversed condition in r8712_usb_read_port()
>
> regards,
> dan carpenter
>
Hi Dan,
Thank you for the review. I will study the code and make sure that your
intuition is correct, which initially it sounds to be.
Thanks,
Luis
next prev parent reply other threads:[~2015-10-24 11:56 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-19 17:10 [PATCH v2 0/3] staging: code cleanup Luis de Bethencourt
2015-10-19 17:14 ` [PATCH v2 1/3] staging: rtl8712: Remove boolean comparisons Luis de Bethencourt
2015-10-22 19:05 ` Dan Carpenter
2015-10-24 11:56 ` Luis de Bethencourt [this message]
2015-10-19 17:15 ` [PATCH v2 2/3] staging: rtl8712: braces should be used on all arms Luis de Bethencourt
2015-10-19 17:16 ` [PATCH v2 3/3] staging: rtl8712: spaces preferred around operands Luis de Bethencourt
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=562B7201.6060604@osg.samsung.com \
--to=luisbg@osg.samsung.com \
--cc=Julia.Lawall@lip6.fr \
--cc=Larry.Finger@lwfinger.net \
--cc=carlos@cgarcia.org \
--cc=cristina.opriceana@gmail.com \
--cc=dan.carpenter@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=fabf@skynet.be \
--cc=florian.c.schilhabel@googlemail.com \
--cc=gregkh@linuxfoundation.org \
--cc=jteki@openedev.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luca@lucaceresoli.net \
--cc=punitvara@gmail.com \
--cc=shivanib134@gmail.com \
--cc=stillcompiling@gmail.com \
--cc=sudipm.mukherjee@gmail.com \
--cc=vthakkar1994@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.