From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752083AbbJXL45 (ORCPT ); Sat, 24 Oct 2015 07:56:57 -0400 Received: from lists.s-osg.org ([54.187.51.154]:53081 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751635AbbJXL44 (ORCPT ); Sat, 24 Oct 2015 07:56:56 -0400 Message-ID: <562B7201.6060604@osg.samsung.com> Date: Sat, 24 Oct 2015 12:56:49 +0100 From: Luis de Bethencourt User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.8.0 MIME-Version: 1.0 To: Dan Carpenter 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 References: <1445274654-10103-1-git-send-email-luisbg@osg.samsung.com> <1445274869-10566-1-git-send-email-luisbg@osg.samsung.com> <20151022190524.GP7340@mwanda> In-Reply-To: <20151022190524.GP7340@mwanda> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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