Message ID | 1400223103.30884.5.camel@joe-AO725 |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
> -----Original Message----- > From: netdev-owner@vger.kernel.org [mailto:netdev- > owner@vger.kernel.org] On Behalf Of Joe Perches > Sent: Friday, May 16, 2014 9:52 AM > To: Ariel Elior; Dmitry Kravkov > Cc: netdev; linux-kernel > Subject: [PATCH] bnx2x: Convert return 0 to return rc > > These "return 0;" uses seem wrong as there are > rc variables where error return values are set > but unused. > > Signed-off-by: Joe Perches <joe@perches.com> > --- > > Hey Ariel/Dmitry > > I've no idea what's right here, but the current > code seems wrong. (or at least under-commented) > > It seems Dmitry changed these, perhaps to make > it more readable, but the commit log isn't very > instructive. > > commit 1d6f3cd8988822c7bdc3c685fac0a99315e83400 > Author: Dmitry Kravkov <dmitry@broadcom.com> > Date: Wed Mar 27 01:05:17 2013 +0000 > > bnx2x: Prevent VF race > > The mail box containing the Vf-Pf messages is susceptible > to a race - it's possible for 2 flows to try and write commands, > causing one to override the other's message. > Use a mutex to synchronize the access, preventing said race. > > drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 2 +- > drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c > b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c > index 81cc2d9..b8078d5 100644 > --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c > +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c > @@ -2695,7 +2695,7 @@ out: > bnx2x_unlock_vf_pf_channel(bp, vf, > CHANNEL_TLV_PF_SET_MAC); > } > > - return 0; > + return rc; > } > > int bnx2x_set_vf_vlan(struct net_device *dev, int vfidx, u16 vlan, u8 qos) > diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c > b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c > index 0c067e8..784c715 100644 > --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c > +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c > @@ -747,7 +747,7 @@ int bnx2x_vfpf_config_mac(struct bnx2x *bp, u8 > *addr, u8 vf_qid, bool set) > out: > bnx2x_vfpf_finalize(bp, &req->first_tlv); > > - return 0; > + return rc; > } > > /* request pf to config rss table for vf queues*/ > Thanks Joe for catching this! Acked-by: Dmitry Kravkov <Dmitry.Kravkov@qlogic.com>
On Fri, 2014-05-16 at 12:02 +0000, Dmitry Kravkov wrote: > > -----Original Message----- > > From: netdev-owner@vger.kernel.org [mailto:netdev- > > owner@vger.kernel.org] On Behalf Of Joe Perches > > Sent: Friday, May 16, 2014 9:52 AM > > To: Ariel Elior; Dmitry Kravkov > > Cc: netdev; linux-kernel > > Subject: [PATCH] bnx2x: Convert return 0 to return rc > > > > These "return 0;" uses seem wrong as there are > > rc variables where error return values are set > > but unused. [] > Thanks Joe for catching this! > > Acked-by: Dmitry Kravkov <Dmitry.Kravkov@qlogic.com> Hello Dmitry. Couple things actually: o Could you please update the MAINTAINER entry for BNX2X? Ariel Elior's email address is still listed as @broadcom and that seems to bounce. o I found this via coccinelle actually. Julia Lawall submitted a patch to remove unused netdev_priv calls. I modified that to check for any expression that returns an unused value and found this and many others. There are a lot of false positives though. @@ local idexpression x; expression E; @@ -x = E; ... when != x -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Joe Perches <joe@perches.com> Date: Fri, 16 May 2014 13:12:24 -0700 > Couple things actually: > o Could you please update the MAINTAINER entry for > BNX2X? Ariel Elior's email address is still listed > as @broadcom and that seems to bounce. Let's please give the Broadcom folks a reasonable opportunity to update the MAINTAINERS entry, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Joe Perches <joe@perches.com> Date: Thu, 15 May 2014 23:51:43 -0700 > These "return 0;" uses seem wrong as there are > rc variables where error return values are set > but unused. > > Signed-off-by: Joe Perches <joe@perches.com> Applied, thanks Joe. -- To unsubscribe from this list: send the line "unsubscribe netdev" 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/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c index 81cc2d9..b8078d5 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c @@ -2695,7 +2695,7 @@ out: bnx2x_unlock_vf_pf_channel(bp, vf, CHANNEL_TLV_PF_SET_MAC); } - return 0; + return rc; } int bnx2x_set_vf_vlan(struct net_device *dev, int vfidx, u16 vlan, u8 qos) diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c index 0c067e8..784c715 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c @@ -747,7 +747,7 @@ int bnx2x_vfpf_config_mac(struct bnx2x *bp, u8 *addr, u8 vf_qid, bool set) out: bnx2x_vfpf_finalize(bp, &req->first_tlv); - return 0; + return rc; } /* request pf to config rss table for vf queues*/
These "return 0;" uses seem wrong as there are rc variables where error return values are set but unused. Signed-off-by: Joe Perches <joe@perches.com> --- Hey Ariel/Dmitry I've no idea what's right here, but the current code seems wrong. (or at least under-commented) It seems Dmitry changed these, perhaps to make it more readable, but the commit log isn't very instructive. commit 1d6f3cd8988822c7bdc3c685fac0a99315e83400 Author: Dmitry Kravkov <dmitry@broadcom.com> Date: Wed Mar 27 01:05:17 2013 +0000 bnx2x: Prevent VF race The mail box containing the Vf-Pf messages is susceptible to a race - it's possible for 2 flows to try and write commands, causing one to override the other's message. Use a mutex to synchronize the access, preventing said race. drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 2 +- drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html