From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrzej Hajda Subject: Re: [PATCH] cxgb4: fix invalid checks in alloc_uld_rxqs Date: Tue, 23 Aug 2016 10:01:59 +0200 Message-ID: <56a89d7c-3a0b-d270-99c9-1632320c6339@samsung.com> References: <1471932979-17864-1-git-send-email-a.hajda@samsung.com> <20160823074554.GA19307@hari-Latitude-E5550> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: "open list:CXGB4 ETHERNET DRIVER (CXGB4)" , Bartlomiej Zolnierkiewicz , Marek Szyprowski , open list To: Hariprasad Shenai Return-path: In-reply-to: <20160823074554.GA19307@hari-Latitude-E5550> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 08/23/2016 09:46 AM, Hariprasad Shenai wrote: > On Tuesday, August 08/23/16, 2016 at 08:16:19 +0200, Andrzej Hajda wrote: >> Local variable msi_idx defined as unsigned int is always >= 0, thus both >> 'if' checks are always true. On the other side presence of USING_MSIX flag >> suggests the checks should not be trivially true. >> The simplest solution is to replace incorrect checks with direct testing >> of adap->flags and remove spare variables. >> >> The problem has been detected using semantic patch >> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci >> > The correct fix is to have local variable 'msi_idx' as int instead of unsigned > int. Thanks for reporting the issue. Do you want me to send a V2? > > Thanks, > Hari > > If adap->flags is constant during the call I see no point in creating separate variable with complicated initialization used only for the same thing as 'adap->flags & USING_MSIX', and even if adap->flags changes during the call much simpler would be to use local var: int using_msix = adap->flags & USING_MSIX; and later use tests: if (using_msix) ... Am I correct? Regards Andrzej