From patchwork Fri Oct 20 07:35:51 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Joe Perches X-Patchwork-Id: 10019085 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9398860224 for ; Fri, 20 Oct 2017 07:35:58 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 868BF28EAF for ; Fri, 20 Oct 2017 07:35:58 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7B7AC28ECC; Fri, 20 Oct 2017 07:35:58 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CFB3528EAF for ; Fri, 20 Oct 2017 07:35:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751967AbdJTHf4 (ORCPT ); Fri, 20 Oct 2017 03:35:56 -0400 Received: from smtprelay0080.hostedemail.com ([216.40.44.80]:36732 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751783AbdJTHfz (ORCPT ); Fri, 20 Oct 2017 03:35:55 -0400 Received: from filter.hostedemail.com (clb03-v110.bra.tucows.net [216.40.38.60]) by smtprelay04.hostedemail.com (Postfix) with ESMTP id 1CD3F180A8144; Fri, 20 Oct 2017 07:35:54 +0000 (UTC) X-Session-Marker: 6A6F6540706572636865732E636F6D X-HE-Tag: spade60_8aa0a1f7fe13d X-Filterd-Recvd-Size: 5457 Received: from XPS-9350 (unknown [47.151.150.235]) (Authenticated sender: joe@perches.com) by omf13.hostedemail.com (Postfix) with ESMTPA; Fri, 20 Oct 2017 07:35:52 +0000 (UTC) Message-ID: <1508484951.6806.46.camel@perches.com> Subject: Re: [PATCH] IB/qib: remove redundant setting of any in for-loop From: Joe Perches To: Colin King , Mike Marciniszyn , Doug Ledford , Sean Hefty , Hal Rosenstock , linux-rdma@vger.kernel.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org Date: Fri, 20 Oct 2017 00:35:51 -0700 In-Reply-To: <20171020072103.10337-1-colin.king@canonical.com> References: <20171020072103.10337-1-colin.king@canonical.com> X-Mailer: Evolution 3.22.6-1ubuntu1 Mime-Version: 1.0 Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, 2017-10-20 at 09:21 +0200, Colin King wrote: > From: Colin Ian King > > The variable all is being set but is never read after this > hence it can be removed from the for loop initialization. > Cleans up clang warning: any is really used as bool and is initialized at function entry. The earlier loop also reinitializes any unnecessarily. > drivers/infiniband/hw/qib/qib_file_ops.c:640:7: warning: Value > stored to 'any' is never read > > Signed-off-by: Colin Ian King > --- > drivers/infiniband/hw/qib/qib_file_ops.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c > index 2d6a191afec0..b5c2e4223ee7 100644 > --- a/drivers/infiniband/hw/qib/qib_file_ops.c > +++ b/drivers/infiniband/hw/qib/qib_file_ops.c > @@ -637,7 +637,7 @@ static int qib_set_part_key(struct qib_ctxtdata *rcd, u16 key) > ret = -EBUSY; > goto bail; > } > - for (any = i = 0; i < ARRAY_SIZE(ppd->pkeys); i++) { > + for (i = 0; i < ARRAY_SIZE(ppd->pkeys); i++) { > if (!ppd->pkeys[i] && > atomic_inc_return(&ppd->pkeyrefs[i]) == 1) { > rcd->pkeys[pidx] = key; Perhaps the function is better written without the empty bail: label and without setting ret and just using return. Combining the int/bool conversion of any and the direct returns seems clearer to me. Something like: ---  drivers/infiniband/hw/qib/qib_file_ops.c | 70 ++++++++++++--------------------  1 file changed, 27 insertions(+), 43 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c index 2d6a191afec0..8078854e1cd6 100644 --- a/drivers/infiniband/hw/qib/qib_file_ops.c +++ b/drivers/infiniband/hw/qib/qib_file_ops.c @@ -568,20 +568,17 @@ static int qib_tid_free(struct qib_ctxtdata *rcd, unsigned subctxt,  static int qib_set_part_key(struct qib_ctxtdata *rcd, u16 key)  {   struct qib_pportdata *ppd = rcd->ppd; - int i, any = 0, pidx = -1; + int i; + bool any = false; + int pidx = -1;   u16 lkey = key & 0x7FFF; - int ret;   - if (lkey == (QIB_DEFAULT_P_KEY & 0x7FFF)) { - /* nothing to do; this key always valid */ - ret = 0; - goto bail; - } + /* nothing to do; this key always valid */ + if (lkey == (QIB_DEFAULT_P_KEY & 0x7FFF)) + return 0;   - if (!lkey) { - ret = -EINVAL; - goto bail; - } + if (!lkey) + return -EINVAL;     /*    * Set the full membership bit, because it has to be @@ -594,18 +591,15 @@ static int qib_set_part_key(struct qib_ctxtdata *rcd, u16 key)   for (i = 0; i < ARRAY_SIZE(rcd->pkeys); i++) {   if (!rcd->pkeys[i] && pidx == -1)   pidx = i; - if (rcd->pkeys[i] == key) { - ret = -EEXIST; - goto bail; - } - } - if (pidx == -1) { - ret = -EBUSY; - goto bail; + if (rcd->pkeys[i] == key) + return -EEXIST;   } - for (any = i = 0; i < ARRAY_SIZE(ppd->pkeys); i++) { + if (pidx == -1) + return -EBUSY; + + for (i = 0; i < ARRAY_SIZE(ppd->pkeys); i++) {   if (!ppd->pkeys[i]) { - any++; + any = true;   continue;   }   if (ppd->pkeys[i] == key) { @@ -613,44 +607,34 @@ static int qib_set_part_key(struct qib_ctxtdata *rcd, u16 key)     if (atomic_inc_return(pkrefs) > 1) {   rcd->pkeys[pidx] = key; - ret = 0; - goto bail; - } else { - /* -  * lost race, decrement count, catch below -  */ - atomic_dec(pkrefs); - any++; + return 0;   } + /* lost race, decrement count, catch below */ + atomic_dec(pkrefs); + any = true;   } - if ((ppd->pkeys[i] & 0x7FFF) == lkey) { + if ((ppd->pkeys[i] & 0x7FFF) == lkey)   /*    * It makes no sense to have both the limited and    * full membership PKEY set at the same time since    * the unlimited one will disable the limited one.    */ - ret = -EEXIST; - goto bail; - } - } - if (!any) { - ret = -EBUSY; - goto bail; + return -EEXIST;   } - for (any = i = 0; i < ARRAY_SIZE(ppd->pkeys); i++) { + if (!any) + return -EBUSY; + + for (i = 0; i < ARRAY_SIZE(ppd->pkeys); i++) {   if (!ppd->pkeys[i] &&       atomic_inc_return(&ppd->pkeyrefs[i]) == 1) {   rcd->pkeys[pidx] = key;   ppd->pkeys[i] = key;   (void) ppd->dd->f_set_ib_cfg(ppd, QIB_IB_CFG_PKEYS, 0); - ret = 0; - goto bail; + return 0;   }   } - ret = -EBUSY;   -bail: - return ret; + return -EBUSY;  }    /**