From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yuval Shaia Subject: Re: [infiniband-hw-i40iw] question about identical code for different branches Date: Thu, 18 May 2017 09:00:39 +0300 Message-ID: <20170518060038.GB6326@yuval-lap> References: <20170517170654.Horde.cfktFjC4G4wPJvJ8X1ZyUvW@gator4166.hostgator.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20170517170654.Horde.cfktFjC4G4wPJvJ8X1ZyUvW-fU+oOHjIBR1LoJgMfuPDHBfZZeVsHd8q@public.gmane.org> Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: "Gustavo A. R. Silva" Cc: Faisal Latif , Shiraz Saleem , Doug Ledford , Sean Hefty , Hal Rosenstock , linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-rdma@vger.kernel.org On Wed, May 17, 2017 at 05:06:54PM -0500, Gustavo A. R. Silva wrote: > > Hello everybody, > > While looking into Coverity ID 1362263 I ran into the following piece of > code at drivers/infiniband/hw/i40iw/i40iw_virtchnl.c:445: > > 445 if (vchnl_msg->iw_op_code == I40IW_VCHNL_OP_GET_VER) { > 446 if (vchnl_msg->iw_op_ver != I40IW_VCHNL_OP_GET_VER_V0) > 447 vchnl_pf_send_get_ver_resp(dev, vf_id, vchnl_msg); > 448 else > 449 vchnl_pf_send_get_ver_resp(dev, vf_id, vchnl_msg); > 450 return I40IW_SUCCESS; > 451 } > > The issue is that lines of code 447 and 449 are identical for different > branches. > > My question here is if one of the branches should be modified, or the entire > _if_ statement replaced? > > Maybe a patch like the following could be applied: > > index f4d1368..48fd327 100644 > --- a/drivers/infiniband/hw/i40iw/i40iw_virtchnl.c > +++ b/drivers/infiniband/hw/i40iw/i40iw_virtchnl.c > @@ -443,10 +443,7 @@ enum i40iw_status_code i40iw_vchnl_recv_pf(struct > i40iw_sc_dev *dev, > if (!dev->vchnl_up) > return I40IW_ERR_NOT_READY; > if (vchnl_msg->iw_op_code == I40IW_VCHNL_OP_GET_VER) { > - if (vchnl_msg->iw_op_ver != I40IW_VCHNL_OP_GET_VER_V0) > - vchnl_pf_send_get_ver_resp(dev, vf_id, vchnl_msg); > - else > - vchnl_pf_send_get_ver_resp(dev, vf_id, vchnl_msg); > + vchnl_pf_send_get_ver_resp(dev, vf_id, vchnl_msg); > return I40IW_SUCCESS; > } > for (iw_vf_idx = 0; iw_vf_idx < I40IW_MAX_PE_ENABLED_VF_COUNT; > iw_vf_idx++) { > > What do you think? This looks like a nice catch! Reviewed-by: Yuval Shaia > > I'd really appreciate any comment on this. > > Thank you! > -- > Gustavo A. R. Silva > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754819AbdERGBN (ORCPT ); Thu, 18 May 2017 02:01:13 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:37171 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754660AbdERGBM (ORCPT ); Thu, 18 May 2017 02:01:12 -0400 Date: Thu, 18 May 2017 09:00:39 +0300 From: Yuval Shaia To: "Gustavo A. R. Silva" Cc: Faisal Latif , Shiraz Saleem , Doug Ledford , Sean Hefty , Hal Rosenstock , linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [infiniband-hw-i40iw] question about identical code for different branches Message-ID: <20170518060038.GB6326@yuval-lap> References: <20170517170654.Horde.cfktFjC4G4wPJvJ8X1ZyUvW@gator4166.hostgator.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170517170654.Horde.cfktFjC4G4wPJvJ8X1ZyUvW@gator4166.hostgator.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, May 17, 2017 at 05:06:54PM -0500, Gustavo A. R. Silva wrote: > > Hello everybody, > > While looking into Coverity ID 1362263 I ran into the following piece of > code at drivers/infiniband/hw/i40iw/i40iw_virtchnl.c:445: > > 445 if (vchnl_msg->iw_op_code == I40IW_VCHNL_OP_GET_VER) { > 446 if (vchnl_msg->iw_op_ver != I40IW_VCHNL_OP_GET_VER_V0) > 447 vchnl_pf_send_get_ver_resp(dev, vf_id, vchnl_msg); > 448 else > 449 vchnl_pf_send_get_ver_resp(dev, vf_id, vchnl_msg); > 450 return I40IW_SUCCESS; > 451 } > > The issue is that lines of code 447 and 449 are identical for different > branches. > > My question here is if one of the branches should be modified, or the entire > _if_ statement replaced? > > Maybe a patch like the following could be applied: > > index f4d1368..48fd327 100644 > --- a/drivers/infiniband/hw/i40iw/i40iw_virtchnl.c > +++ b/drivers/infiniband/hw/i40iw/i40iw_virtchnl.c > @@ -443,10 +443,7 @@ enum i40iw_status_code i40iw_vchnl_recv_pf(struct > i40iw_sc_dev *dev, > if (!dev->vchnl_up) > return I40IW_ERR_NOT_READY; > if (vchnl_msg->iw_op_code == I40IW_VCHNL_OP_GET_VER) { > - if (vchnl_msg->iw_op_ver != I40IW_VCHNL_OP_GET_VER_V0) > - vchnl_pf_send_get_ver_resp(dev, vf_id, vchnl_msg); > - else > - vchnl_pf_send_get_ver_resp(dev, vf_id, vchnl_msg); > + vchnl_pf_send_get_ver_resp(dev, vf_id, vchnl_msg); > return I40IW_SUCCESS; > } > for (iw_vf_idx = 0; iw_vf_idx < I40IW_MAX_PE_ENABLED_VF_COUNT; > iw_vf_idx++) { > > What do you think? This looks like a nice catch! Reviewed-by: Yuval Shaia > > I'd really appreciate any comment on this. > > Thank you! > -- > Gustavo A. R. Silva > > > > > -- > 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