diff mbox

bnx2x: Convert return 0 to return rc

Message ID 1400223103.30884.5.camel@joe-AO725
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Joe Perches May 16, 2014, 6:51 a.m. UTC
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

Comments

Dmitry Kravkov May 16, 2014, 12:02 p.m. UTC | #1
> -----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>
Joe Perches May 16, 2014, 8:12 p.m. UTC | #2
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
David Miller May 16, 2014, 8:49 p.m. UTC | #3
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
David Miller May 19, 2014, 12:51 a.m. UTC | #4
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 mbox

Patch

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*/