diff mbox

[net-next] cxgb4: Fix static checker warnings

Message ID 1405451843-3081-1-git-send-email-anish@chelsio.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Anish Bhatt July 15, 2014, 7:17 p.m. UTC
This patch fixes static checker warnings as pointed out on by Dan Carpenter

Fixes: 76bcb31efc06 ("cxgb4 : Add DCBx support codebase  and dcbnl_ops")
Signed-off-by: Anish Bhatt <anish@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Casey Leedom July 15, 2014, 9:57 p.m. UTC | #1
While it's great to fix these warnings, it seems like this might be 
going in the wrong direction.

   Currently only the cxgb4 and bnx2x drivers offer (struct 
dcbnl_rtnl_ops).setapp() calls.  The bnx2x driver also returns -EINVAL, 
etc.  Meanwhile, the only call to the .setapp() entry is in 
dcbnl_setapp() and it stores the result back into an int "ret".  It 
strikes me that the real bug may be an incorrect "u8" return type for 
the .setapp() prototype in include/net/dcbnl.h.  The prototype was added 
in commit 7114323b almost 5 years ago so I'm not sure if the original 
author, Yi Zou <yi.zou@intel.com>, will be around to offer backing 
information on this ...

Casey

On 07/15/14 12:17, Anish Bhatt wrote:
> This patch fixes static checker warnings as pointed out on by Dan Carpenter
>
> Fixes: 76bcb31efc06 ("cxgb4 : Add DCBx support codebase  and dcbnl_ops")
> Signed-off-by: Anish Bhatt <anish@chelsio.com>
> ---
>   drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
> index a8b1073e6373..8734dfe0da32 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
> @@ -676,15 +676,15 @@ static u8 cxgb4_setapp(struct net_device *dev, u8 app_idtype, u16 app_id,
>   
>   
>   	if (pi->dcb.state != CXGB4_DCB_STATE_FW_ALLSYNCED)
> -		return -EINVAL;
> +		return 0;
>   
>   	/* DCB info gets thrown away on link up */
>   	if (!netif_carrier_ok(dev))
> -		return -ENOLINK;
> +		return 0;
>   
>   	if (app_idtype != DCB_APP_IDTYPE_ETHTYPE &&
>   	    app_idtype != DCB_APP_IDTYPE_PORTNUM)
> -		return -EINVAL;
> +		return 0;
>   
>   	for (i = 0; i < CXGB4_MAX_DCBX_APP_SUPPORTED; i++) {
>   		INIT_PORT_DCB_READ_LOCAL_CMD(pcmd, pi->port_id);
> @@ -710,7 +710,7 @@ static u8 cxgb4_setapp(struct net_device *dev, u8 app_idtype, u16 app_id,
>   	if (i == CXGB4_MAX_DCBX_APP_SUPPORTED) {
>   		/* no empty slots available */
>   		dev_err(adap->pdev_dev, "DCB app table full\n");
> -		return -EBUSY;
> +		return 0;
>   	}
>   
>   	/* write out new app table entry */

--
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 July 16, 2014, 10:45 p.m. UTC | #2
From: Anish Bhatt <anish@chelsio.com>
Date: Tue, 15 Jul 2014 12:17:23 -0700

> This patch fixes static checker warnings as pointed out on by Dan Carpenter
> 
> Fixes: 76bcb31efc06 ("cxgb4 : Add DCBx support codebase  and dcbnl_ops")
> Signed-off-by: Anish Bhatt <anish@chelsio.com>

The real bug here is more likely, as Casey Leedom said, that the method
returns u8 instead of int.

The only other implementation of this method, bnx2x, also tries to return
negative error codes.

So please just change the return type in the method signature and the
two implementations.

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
diff mbox

Patch

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
index a8b1073e6373..8734dfe0da32 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
@@ -676,15 +676,15 @@  static u8 cxgb4_setapp(struct net_device *dev, u8 app_idtype, u16 app_id,
 
 
 	if (pi->dcb.state != CXGB4_DCB_STATE_FW_ALLSYNCED)
-		return -EINVAL;
+		return 0;
 
 	/* DCB info gets thrown away on link up */
 	if (!netif_carrier_ok(dev))
-		return -ENOLINK;
+		return 0;
 
 	if (app_idtype != DCB_APP_IDTYPE_ETHTYPE &&
 	    app_idtype != DCB_APP_IDTYPE_PORTNUM)
-		return -EINVAL;
+		return 0;
 
 	for (i = 0; i < CXGB4_MAX_DCBX_APP_SUPPORTED; i++) {
 		INIT_PORT_DCB_READ_LOCAL_CMD(pcmd, pi->port_id);
@@ -710,7 +710,7 @@  static u8 cxgb4_setapp(struct net_device *dev, u8 app_idtype, u16 app_id,
 	if (i == CXGB4_MAX_DCBX_APP_SUPPORTED) {
 		/* no empty slots available */
 		dev_err(adap->pdev_dev, "DCB app table full\n");
-		return -EBUSY;
+		return 0;
 	}
 
 	/* write out new app table entry */