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

Groups > linux.kernel > #1402663 > unrolled thread

[PATCH 1/1] net: pegasus: remove dead coding

Started byHeinrich Schuchardt <xypron.glpk@gmx.de>
First post2016-05-18 02:20 +0200
Last post2016-05-20 18:40 +0200
Articles 11 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/1] net: pegasus: remove dead coding Heinrich Schuchardt <xypron.glpk@gmx.de> - 2016-05-18 02:20 +0200
    Re: [PATCH 1/1] net: pegasus: remove dead coding Petko Manolov <petkan@mip-labs.com> - 2016-05-18 08:30 +0200
    Re: [1/1] net: pegasus: remove dead coding Guenter Roeck <linux@roeck-us.net> - 2016-05-18 08:40 +0200
      Re: [1/1] net: pegasus: remove dead coding Oliver Neukum <oneukum@suse.com> - 2016-05-18 09:20 +0200
        Re: [1/1] net: pegasus: remove dead coding Petko Manolov <petkan@mip-labs.com> - 2016-05-18 10:00 +0200
        [PATCH 1/1] net: pegasus: simplify logical constraint Heinrich Schuchardt <xypron.glpk@gmx.de> - 2016-05-18 20:50 +0200
          Re: [PATCH 1/1] net: pegasus: simplify logical constraint Petko Manolov <petkan@mip-labs.com> - 2016-05-19 08:00 +0200
          Re: [PATCH 1/1] net: pegasus: simplify logical constraint David Miller <davem@davemloft.net> - 2016-05-23 22:50 +0200
    Re: [PATCH 1/1] net: pegasus: remove dead coding David Miller <davem@davemloft.net> - 2016-05-19 20:40 +0200
      Re: [PATCH 1/1] net: pegasus: remove dead coding Petko Manolov <petkan@mip-labs.com> - 2016-05-20 09:40 +0200
        Re: [PATCH 1/1] net: pegasus: remove dead coding David Miller <davem@davemloft.net> - 2016-05-20 18:40 +0200

#1402663 — [PATCH 1/1] net: pegasus: remove dead coding

FromHeinrich Schuchardt <xypron.glpk@gmx.de>
Date2016-05-18 02:20 +0200
Subject[PATCH 1/1] net: pegasus: remove dead coding
Message-ID<rzXns-35d-13@gated-at.bofh.it>
(!count || count < 4) is always true.
So let's remove the coding which is dead at least since 2005.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 drivers/net/usb/pegasus.c | 53 -----------------------------------------------
 1 file changed, 53 deletions(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 36cd7f0..1903d2e 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -470,61 +470,8 @@ static void read_bulk_callback(struct urb *urb)
 		return;
 	default:
 		netif_dbg(pegasus, rx_err, net, "RX status %d\n", status);
-		goto goon;
 	}
 
-	if (!count || count < 4)
-		goto goon;
-
-	rx_status = buf[count - 2];
-	if (rx_status & 0x1e) {
-		netif_dbg(pegasus, rx_err, net,
-			  "RX packet error %x\n", rx_status);
-		pegasus->stats.rx_errors++;
-		if (rx_status & 0x06)	/* long or runt	*/
-			pegasus->stats.rx_length_errors++;
-		if (rx_status & 0x08)
-			pegasus->stats.rx_crc_errors++;
-		if (rx_status & 0x10)	/* extra bits	*/
-			pegasus->stats.rx_frame_errors++;
-		goto goon;
-	}
-	if (pegasus->chip == 0x8513) {
-		pkt_len = le32_to_cpu(*(__le32 *)urb->transfer_buffer);
-		pkt_len &= 0x0fff;
-		pegasus->rx_skb->data += 2;
-	} else {
-		pkt_len = buf[count - 3] << 8;
-		pkt_len += buf[count - 4];
-		pkt_len &= 0xfff;
-		pkt_len -= 4;
-	}
-
-	/*
-	 * If the packet is unreasonably long, quietly drop it rather than
-	 * kernel panicing by calling skb_put.
-	 */
-	if (pkt_len > PEGASUS_MTU)
-		goto goon;
-
-	/*
-	 * at this point we are sure pegasus->rx_skb != NULL
-	 * so we go ahead and pass up the packet.
-	 */
-	skb_put(pegasus->rx_skb, pkt_len);
-	pegasus->rx_skb->protocol = eth_type_trans(pegasus->rx_skb, net);
-	netif_rx(pegasus->rx_skb);
-	pegasus->stats.rx_packets++;
-	pegasus->stats.rx_bytes += pkt_len;
-
-	if (pegasus->flags & PEGASUS_UNPLUG)
-		return;
-
-	pegasus->rx_skb = __netdev_alloc_skb_ip_align(pegasus->net, PEGASUS_MTU,
-						      GFP_ATOMIC);
-
-	if (pegasus->rx_skb == NULL)
-		goto tl_sched;
 goon:
 	usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
 			  usb_rcvbulkpipe(pegasus->usb, 1),
-- 
2.1.4

[toc] | [next] | [standalone]

#1402737

FromPetko Manolov <petkan@mip-labs.com>
Date2016-05-18 08:30 +0200
Message-ID<rA39w-6Ng-15@gated-at.bofh.it>
In reply to#1402663
On 16-05-18 02:13:30, Heinrich Schuchardt wrote:
> (!count || count < 4) is always true.
> So let's remove the coding which is dead at least since 2005.

You may want to reconsider the above statement.  Just assume that 'count' is 
typically between 56 and 1514 bytes.


		Petko


> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  drivers/net/usb/pegasus.c | 53 -----------------------------------------------
>  1 file changed, 53 deletions(-)
> 
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index 36cd7f0..1903d2e 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -470,61 +470,8 @@ static void read_bulk_callback(struct urb *urb)
>  		return;
>  	default:
>  		netif_dbg(pegasus, rx_err, net, "RX status %d\n", status);
> -		goto goon;
>  	}
>  
> -	if (!count || count < 4)
> -		goto goon;
> -
> -	rx_status = buf[count - 2];
> -	if (rx_status & 0x1e) {
> -		netif_dbg(pegasus, rx_err, net,
> -			  "RX packet error %x\n", rx_status);
> -		pegasus->stats.rx_errors++;
> -		if (rx_status & 0x06)	/* long or runt	*/
> -			pegasus->stats.rx_length_errors++;
> -		if (rx_status & 0x08)
> -			pegasus->stats.rx_crc_errors++;
> -		if (rx_status & 0x10)	/* extra bits	*/
> -			pegasus->stats.rx_frame_errors++;
> -		goto goon;
> -	}
> -	if (pegasus->chip == 0x8513) {
> -		pkt_len = le32_to_cpu(*(__le32 *)urb->transfer_buffer);
> -		pkt_len &= 0x0fff;
> -		pegasus->rx_skb->data += 2;
> -	} else {
> -		pkt_len = buf[count - 3] << 8;
> -		pkt_len += buf[count - 4];
> -		pkt_len &= 0xfff;
> -		pkt_len -= 4;
> -	}
> -
> -	/*
> -	 * If the packet is unreasonably long, quietly drop it rather than
> -	 * kernel panicing by calling skb_put.
> -	 */
> -	if (pkt_len > PEGASUS_MTU)
> -		goto goon;
> -
> -	/*
> -	 * at this point we are sure pegasus->rx_skb != NULL
> -	 * so we go ahead and pass up the packet.
> -	 */
> -	skb_put(pegasus->rx_skb, pkt_len);
> -	pegasus->rx_skb->protocol = eth_type_trans(pegasus->rx_skb, net);
> -	netif_rx(pegasus->rx_skb);
> -	pegasus->stats.rx_packets++;
> -	pegasus->stats.rx_bytes += pkt_len;
> -
> -	if (pegasus->flags & PEGASUS_UNPLUG)
> -		return;
> -
> -	pegasus->rx_skb = __netdev_alloc_skb_ip_align(pegasus->net, PEGASUS_MTU,
> -						      GFP_ATOMIC);
> -
> -	if (pegasus->rx_skb == NULL)
> -		goto tl_sched;
>  goon:
>  	usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
>  			  usb_rcvbulkpipe(pegasus->usb, 1),
> -- 
> 2.1.4
> 
>

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

#1402739 — Re: [1/1] net: pegasus: remove dead coding

FromGuenter Roeck <linux@roeck-us.net>
Date2016-05-18 08:40 +0200
SubjectRe: [1/1] net: pegasus: remove dead coding
Message-ID<rA3jc-6Qx-7@gated-at.bofh.it>
In reply to#1402663
On Wed, May 18, 2016 at 02:13:30AM +0200, Heinrich Schuchardt wrote:
> (!count || count < 4) is always true.

Even if count >= 4 ?

Guenter

> So let's remove the coding which is dead at least since 2005.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> 
> ---
> drivers/net/usb/pegasus.c | 53 -----------------------------------------------
>  1 file changed, 53 deletions(-)
> 
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index 36cd7f0..1903d2e 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -470,61 +470,8 @@ static void read_bulk_callback(struct urb *urb)
>  		return;
>  	default:
>  		netif_dbg(pegasus, rx_err, net, "RX status %d\n", status);
> -		goto goon;
>  	}
>  
> -	if (!count || count < 4)
> -		goto goon;
> -
> -	rx_status = buf[count - 2];
> -	if (rx_status & 0x1e) {
> -		netif_dbg(pegasus, rx_err, net,
> -			  "RX packet error %x\n", rx_status);
> -		pegasus->stats.rx_errors++;
> -		if (rx_status & 0x06)	/* long or runt	*/
> -			pegasus->stats.rx_length_errors++;
> -		if (rx_status & 0x08)
> -			pegasus->stats.rx_crc_errors++;
> -		if (rx_status & 0x10)	/* extra bits	*/
> -			pegasus->stats.rx_frame_errors++;
> -		goto goon;
> -	}
> -	if (pegasus->chip == 0x8513) {
> -		pkt_len = le32_to_cpu(*(__le32 *)urb->transfer_buffer);
> -		pkt_len &= 0x0fff;
> -		pegasus->rx_skb->data += 2;
> -	} else {
> -		pkt_len = buf[count - 3] << 8;
> -		pkt_len += buf[count - 4];
> -		pkt_len &= 0xfff;
> -		pkt_len -= 4;
> -	}
> -
> -	/*
> -	 * If the packet is unreasonably long, quietly drop it rather than
> -	 * kernel panicing by calling skb_put.
> -	 */
> -	if (pkt_len > PEGASUS_MTU)
> -		goto goon;
> -
> -	/*
> -	 * at this point we are sure pegasus->rx_skb != NULL
> -	 * so we go ahead and pass up the packet.
> -	 */
> -	skb_put(pegasus->rx_skb, pkt_len);
> -	pegasus->rx_skb->protocol = eth_type_trans(pegasus->rx_skb, net);
> -	netif_rx(pegasus->rx_skb);
> -	pegasus->stats.rx_packets++;
> -	pegasus->stats.rx_bytes += pkt_len;
> -
> -	if (pegasus->flags & PEGASUS_UNPLUG)
> -		return;
> -
> -	pegasus->rx_skb = __netdev_alloc_skb_ip_align(pegasus->net, PEGASUS_MTU,
> -						      GFP_ATOMIC);
> -
> -	if (pegasus->rx_skb == NULL)
> -		goto tl_sched;
>  goon:
>  	usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
>  			  usb_rcvbulkpipe(pegasus->usb, 1),

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

#1402755 — Re: [1/1] net: pegasus: remove dead coding

FromOliver Neukum <oneukum@suse.com>
Date2016-05-18 09:20 +0200
SubjectRe: [1/1] net: pegasus: remove dead coding
Message-ID<rA3VT-7ii-7@gated-at.bofh.it>
In reply to#1402739
On Tue, 2016-05-17 at 23:30 -0700, Guenter Roeck wrote:
> On Wed, May 18, 2016 at 02:13:30AM +0200, Heinrich Schuchardt wrote:
> > (!count || count < 4) is always true.
> 
> Even if count >= 4 ?

The check for !count is redundant, though. Gcc, however,
will surely simplify the expression.

	Regards
		Oliver

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

#1402779 — Re: [1/1] net: pegasus: remove dead coding

FromPetko Manolov <petkan@mip-labs.com>
Date2016-05-18 10:00 +0200
SubjectRe: [1/1] net: pegasus: remove dead coding
Message-ID<rA4yC-7yy-15@gated-at.bofh.it>
In reply to#1402755
On 16-05-18 09:15:40, Oliver Neukum wrote:
> On Tue, 2016-05-17 at 23:30 -0700, Guenter Roeck wrote:
> > On Wed, May 18, 2016 at 02:13:30AM +0200, Heinrich Schuchardt wrote:
> > > (!count || count < 4) is always true.
> > 
> > Even if count >= 4 ?
> 
> The check for !count is redundant, though. Gcc, however,
> will surely simplify the expression.

Yep, gcc-6 generates this code:

	...
	cmp	$0x3,%edx
	jle	b9 <read_bulk_callback+0x69>
	...

Which does not invalidate your statement that "!count" is redundant. :)


		Petko

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

#1403218 — [PATCH 1/1] net: pegasus: simplify logical constraint

FromHeinrich Schuchardt <xypron.glpk@gmx.de>
Date2016-05-18 20:50 +0200
Subject[PATCH 1/1] net: pegasus: simplify logical constraint
Message-ID<rAeHD-5LI-5@gated-at.bofh.it>
In reply to#1402755
If !count is true, count < 4 is also true.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 drivers/net/usb/pegasus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 36cd7f0..9bbe0161 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -473,7 +473,7 @@ static void read_bulk_callback(struct urb *urb)
 		goto goon;
 	}
 
-	if (!count || count < 4)
+	if (count < 4)
 		goto goon;
 
 	rx_status = buf[count - 2];
-- 
2.1.4

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

#1403444 — Re: [PATCH 1/1] net: pegasus: simplify logical constraint

FromPetko Manolov <petkan@mip-labs.com>
Date2016-05-19 08:00 +0200
SubjectRe: [PATCH 1/1] net: pegasus: simplify logical constraint
Message-ID<rApa1-49X-9@gated-at.bofh.it>
In reply to#1403218
On 16-05-18 20:40:51, Heinrich Schuchardt wrote:
> If !count is true, count < 4 is also true.

Yep, you're right.  However, gcc optimizes away the first condition.  What you 
really got me to think about is whether 4 is the right number.  I guess i shall 
refer to the HW documentation.


		Petko


> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  drivers/net/usb/pegasus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index 36cd7f0..9bbe0161 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -473,7 +473,7 @@ static void read_bulk_callback(struct urb *urb)
>  		goto goon;
>  	}
>  
> -	if (!count || count < 4)
> +	if (count < 4)
>  		goto goon;
>  
>  	rx_status = buf[count - 2];
> -- 
> 2.1.4
> 
>

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

#1405620 — Re: [PATCH 1/1] net: pegasus: simplify logical constraint

FromDavid Miller <davem@davemloft.net>
Date2016-05-23 22:50 +0200
SubjectRe: [PATCH 1/1] net: pegasus: simplify logical constraint
Message-ID<rC4Xw-2dT-17@gated-at.bofh.it>
In reply to#1403218
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
Date: Wed, 18 May 2016 20:40:51 +0200

> If !count is true, count < 4 is also true.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied.

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

#1403915

FromDavid Miller <davem@davemloft.net>
Date2016-05-19 20:40 +0200
Message-ID<rAB1w-3od-11@gated-at.bofh.it>
In reply to#1402663
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
Date: Wed, 18 May 2016 02:13:30 +0200

> (!count || count < 4) is always true.
> So let's remove the coding which is dead at least since 2005.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied.

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

#1404199

FromPetko Manolov <petkan@mip-labs.com>
Date2016-05-20 09:40 +0200
Message-ID<rANcm-2D2-13@gated-at.bofh.it>
In reply to#1403915
On 16-05-19 11:35:42, David Miller wrote:
> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Date: Wed, 18 May 2016 02:13:30 +0200
> 
> > (!count || count < 4) is always true.
> > So let's remove the coding which is dead at least since 2005.
> > 
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> 
> Applied.

David, the patch you applied is broken.  It seems that you didn't follow the 
discussion from the past couple of days.  Please revert it.


		Petko

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

#1404596

FromDavid Miller <davem@davemloft.net>
Date2016-05-20 18:40 +0200
Message-ID<rAVCX-7On-37@gated-at.bofh.it>
In reply to#1404199
From: Petko Manolov <petkan@mip-labs.com>
Date: Fri, 20 May 2016 10:33:47 +0300

> On 16-05-19 11:35:42, David Miller wrote:
>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Date: Wed, 18 May 2016 02:13:30 +0200
>> 
>> > (!count || count < 4) is always true.
>> > So let's remove the coding which is dead at least since 2005.
>> > 
>> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> 
>> Applied.
> 
> David, the patch you applied is broken.  It seems that you didn't follow the 
> discussion from the past couple of days.  Please revert it.

I did.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web