Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1404231 > unrolled thread

[PATCH] net: pegasus: remove unused variables and labels

Back to article view | Back to linux.kernel

Contents

  [PATCH] net: pegasus: remove unused variables and labels Arnd Bergmann <arnd@arndb.de> - 2016-05-20 10:30 +0200
    Re: [PATCH] net: pegasus: remove unused variables and labels Petko Manolov <petkan@mip-labs.com> - 2016-05-20 11:40 +0200
      Re: [PATCH] net: pegasus: remove unused variables and labels Arnd Bergmann <arnd@arndb.de> - 2016-05-20 12:10 +0200
    Re: [PATCH] net: pegasus: remove unused variables and labels David Miller <davem@davemloft.net> - 2016-05-20 18:50 +0200

#1404231 — [PATCH] net: pegasus: remove unused variables and labels

The latest dead-code removal was slightly incomplete and
left a few things behind that we now get compiler warnings
for:

drivers/net/usb/pegasus.c: In function 'read_bulk_callback':
drivers/net/usb/pegasus.c:475:1: error: label 'goon' defined but not used [-Werror=unused-label]
drivers/net/usb/pegasus.c:446:8: error: unused variable 'pkt_len' [-Werror=unused-variable]
drivers/net/usb/pegasus.c:445:6: error: unused variable 'buf' [-Werror=unused-variable]
drivers/net/usb/pegasus.c:443:17: error: unused variable 'count' [-Werror=unused-variable]

This removes them as well.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: e00be9e4d0ff ("net: pegasus: remove dead coding")
---
 drivers/net/usb/pegasus.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 1903d2e2b62d..df6d90ab7ca7 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -440,10 +440,8 @@ static void read_bulk_callback(struct urb *urb)
 {
 	pegasus_t *pegasus = urb->context;
 	struct net_device *net;
-	int rx_status, count = urb->actual_length;
+	int rx_status;
 	int status = urb->status;
-	u8 *buf = urb->transfer_buffer;
-	__u16 pkt_len;
 
 	if (!pegasus)
 		return;
@@ -472,7 +470,6 @@ static void read_bulk_callback(struct urb *urb)
 		netif_dbg(pegasus, rx_err, net, "RX status %d\n", status);
 	}
 
-goon:
 	usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
 			  usb_rcvbulkpipe(pegasus->usb, 1),
 			  pegasus->rx_skb->data, PEGASUS_MTU,
-- 
2.7.0

[toc] | [next] | [standalone]

#1404280

Guys, come on.  This code is not dead.  This code is executed every time an 
ethernet packet is received.  It takes care of various error statistics. More 
importantly, it sends the actual (reported by the adapter) packet length to the 
network layer along with the packet.

This patch removes skb_put() and netif_rx() calls and effectively kills the RX 
path.  Not to mention that the driver was not even compiled before sending the 
patch upstream.

The only sensible, although cosmetic, change would be to replace:

	if (!count || count < 4)

with

	if (count < 4)

even though GCC takes care and it optimizes away "!count" condition.

Please revert this patch before Linus pulls from the network tree.


		Petko



On 16-05-20 10:22:30, Arnd Bergmann wrote:
> The latest dead-code removal was slightly incomplete and
> left a few things behind that we now get compiler warnings
> for:
> 
> drivers/net/usb/pegasus.c: In function 'read_bulk_callback':
> drivers/net/usb/pegasus.c:475:1: error: label 'goon' defined but not used [-Werror=unused-label]
> drivers/net/usb/pegasus.c:446:8: error: unused variable 'pkt_len' [-Werror=unused-variable]
> drivers/net/usb/pegasus.c:445:6: error: unused variable 'buf' [-Werror=unused-variable]
> drivers/net/usb/pegasus.c:443:17: error: unused variable 'count' [-Werror=unused-variable]
> 
> This removes them as well.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: e00be9e4d0ff ("net: pegasus: remove dead coding")
> ---
>  drivers/net/usb/pegasus.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index 1903d2e2b62d..df6d90ab7ca7 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -440,10 +440,8 @@ static void read_bulk_callback(struct urb *urb)
>  {
>  	pegasus_t *pegasus = urb->context;
>  	struct net_device *net;
> -	int rx_status, count = urb->actual_length;
> +	int rx_status;
>  	int status = urb->status;
> -	u8 *buf = urb->transfer_buffer;
> -	__u16 pkt_len;
>  
>  	if (!pegasus)
>  		return;
> @@ -472,7 +470,6 @@ static void read_bulk_callback(struct urb *urb)
>  		netif_dbg(pegasus, rx_err, net, "RX status %d\n", status);
>  	}
>  
> -goon:
>  	usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
>  			  usb_rcvbulkpipe(pegasus->usb, 1),
>  			  pegasus->rx_skb->data, PEGASUS_MTU,
> -- 
> 2.7.0
> 
> 

[toc] | [prev] | [next] | [standalone]

#1404308

On Friday 20 May 2016 12:32:23 Petko Manolov wrote:
> Guys, come on.  This code is not dead.  This code is executed every time an 
> ethernet packet is received.  It takes care of various error statistics. More 
> importantly, it sends the actual (reported by the adapter) packet length to the 
> network layer along with the packet.
> 
> This patch removes skb_put() and netif_rx() calls and effectively kills the RX 
> path.  Not to mention that the driver was not even compiled before sending the 
> patch upstream.
> 
> The only sensible, although cosmetic, change would be to replace:
> 
>         if (!count || count < 4)
> 
> with
> 
>         if (count < 4)
> 
> even though GCC takes care and it optimizes away "!count" condition.
> 
> Please revert this patch before Linus pulls from the network tree.
> 

Agreed. I failed to check the commit that introduced the warning for
the more serious problem.

Please revert e00be9e4d0ff, it just makes no sense.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1404604

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 20 May 2016 10:22:30 +0200

> The latest dead-code removal was slightly incomplete and
> left a few things behind that we now get compiler warnings
> for:
> 
> drivers/net/usb/pegasus.c: In function 'read_bulk_callback':
> drivers/net/usb/pegasus.c:475:1: error: label 'goon' defined but not used [-Werror=unused-label]
> drivers/net/usb/pegasus.c:446:8: error: unused variable 'pkt_len' [-Werror=unused-variable]
> drivers/net/usb/pegasus.c:445:6: error: unused variable 'buf' [-Werror=unused-variable]
> drivers/net/usb/pegasus.c:443:17: error: unused variable 'count' [-Werror=unused-variable]
> 
> This removes them as well.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: e00be9e4d0ff ("net: pegasus: remove dead coding")

The patch in question was broken and has been reverted.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web