[PATCH][ATM]: fix sparse checker warnings

[PATCH][ATM]: fix sparse checker warnings

[chas williams (contractor)]

[ATM]: fix sparse checker warnings (from Stephen Hemminger <shemminger@osdl.org>)


diff -Nru a/net/atm/br2684.c b/net/atm/br2684.c
- --- a/net/atm/br2684.c	2004-06-22 14:04:02 -07:00
+++ b/net/atm/br2684.c	2004-06-22 14:04:02 -07:00
@@ -562,7 +562,7 @@
 	atmvcc->push = br2684_push;
 	skb_queue_head_init(&copy);
 	skb_migrate(&atmvcc->sk->sk_receive_queue, &copy);
- -	while ((skb = skb_dequeue(&copy))) {
+	while ((skb = skb_dequeue(&copy)) != NULL) {
 		BRPRIV(skb->dev)->stats.rx_bytes -= skb->len;
 		BRPRIV(skb->dev)->stats.rx_packets--;
 		br2684_push(atmvcc, skb);
diff -Nru a/net/atm/clip.c b/net/atm/clip.c
- --- a/net/atm/clip.c	2004-06-22 14:04:02 -07:00
+++ b/net/atm/clip.c	2004-06-22 14:04:02 -07:00
@@ -503,7 +503,7 @@
 	skb_queue_head_init(&copy);
 	skb_migrate(&vcc->sk->sk_receive_queue, &copy);
 	/* re-process everything received between connection setup and MKIP */
- -	while ((skb = skb_dequeue(&copy)))
+	while ((skb = skb_dequeue(&copy)) != NULL)
 		if (!clip_devs) {
 			atm_return(vcc,skb->truesize);
 			kfree_skb(skb);
diff -Nru a/net/atm/common.c b/net/atm/common.c
- --- a/net/atm/common.c	2004-06-22 14:04:02 -07:00
+++ b/net/atm/common.c	2004-06-22 14:04:02 -07:00
@@ -187,7 +187,7 @@
 
 		vcc_remove_socket(sk);	/* no more receive */
 
- -		while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue))) {
+		while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue)) != NULL) {
 			atm_return(vcc,skb->truesize);
 			kfree_skb(skb);
 		}
diff -Nru a/net/atm/lec.c b/net/atm/lec.c
- --- a/net/atm/lec.c	2004-06-22 14:04:02 -07:00
+++ b/net/atm/lec.c	2004-06-22 14:04:02 -07:00
@@ -567,7 +567,7 @@
         if (skb_peek(&vcc->sk->sk_receive_queue))
 		printk("%s lec_atm_close: closing with messages pending\n",
                        dev->name);
- -        while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue))) {
+        while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue)) != NULL) {
                 atm_return(vcc, skb->truesize);
 		dev_kfree_skb(skb);
         }
@@ -1940,7 +1940,7 @@
                                            priv->path_switching_delay)) {
 			                        struct sk_buff *skb;
 
- - 				                while ((skb = skb_dequeue(&entry->tx_wait)))
+ 				                while ((skb = skb_dequeue(&entry->tx_wait)) != NULL)
 					                lec_send(entry->vcc, skb, entry->priv);
                                                 entry->last_used = jiffies;
                                                 entry->status = 
@@ -2337,7 +2337,7 @@
                             entry->status == ESI_FLUSH_PENDING) {
 			        struct sk_buff *skb;
 
- - 				while ((skb = skb_dequeue(&entry->tx_wait)))
+ 				while ((skb = skb_dequeue(&entry->tx_wait)) != NULL)
 					lec_send(entry->vcc, skb, entry->priv);
                                 entry->status = ESI_FORWARD_DIRECT;
                                 DPRINTK("LEC_ARP: Flushed\n");
diff -Nru a/net/atm/svc.c b/net/atm/svc.c
- --- a/net/atm/svc.c	2004-06-22 14:04:02 -07:00
+++ b/net/atm/svc.c	2004-06-22 14:04:02 -07:00
@@ -66,7 +66,7 @@
 	}
 	/* beware - socket is still in use by atmsigd until the last
 	   as_indicate has been answered */
- -	while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue))) {
+	while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue)) != NULL) {
 		DPRINTK("LISTEN REL\n");
 		sigd_enq2(NULL,as_reject,vcc,NULL,NULL,&vcc->qos,0);
 		dev_kfree_skb(skb);


------- End of Forwarded Message

Re: [PATCH][ATM]: fix sparse checker warnings

[Chris Wedgwood]

On Sat, Jun 26, 2004 at 06:45:50PM -0400, chas williams (contractor) wrote:

> diff -Nru a/net/atm/br2684.c b/net/atm/br2684.c
> - --- a/net/atm/br2684.c	2004-06-22 14:04:02 -07:00

forwarding mangled the patch

> +++ b/net/atm/br2684.c	2004-06-22 14:04:02 -07:00
> @@ -562,7 +562,7 @@
>  	atmvcc->push = br2684_push;
>  	skb_queue_head_init(&copy);
>  	skb_migrate(&atmvcc->sk->sk_receive_queue, &copy);

> - -	while ((skb = skb_dequeue(&copy))) {
> +	while ((skb = skb_dequeue(&copy)) != NULL) {

I know it's a matter of style, but I really hate the 'assignment in
conditional' warning sparse spews out, especially when many people,
myself included really do write while ((a = b)) --- the extra
parentheses as a compromise to keep gcc quiet.


   --cw

Re: [PATCH][ATM]: fix sparse checker warnings

[chas williams (contractor)]

In message <20040626225230.GA12698@taniwha.stupidest.org>,Chris Wedgwood writes
:
>forwarding mangled the patch

a bit of brillant work on my part.  here it is again but undamaged.

[PATCH][ATM]: fix sparse checker warnings (by Stephen Hemminger <shemminger@osdl.org>)

diff -Nru a/net/atm/br2684.c b/net/atm/br2684.c
--- a/net/atm/br2684.c	2004-06-22 14:04:02 -07:00
+++ b/net/atm/br2684.c	2004-06-22 14:04:02 -07:00
@@ -562,7 +562,7 @@
 	atmvcc->push = br2684_push;
 	skb_queue_head_init(&copy);
 	skb_migrate(&atmvcc->sk->sk_receive_queue, &copy);
-	while ((skb = skb_dequeue(&copy))) {
+	while ((skb = skb_dequeue(&copy)) != NULL) {
 		BRPRIV(skb->dev)->stats.rx_bytes -= skb->len;
 		BRPRIV(skb->dev)->stats.rx_packets--;
 		br2684_push(atmvcc, skb);
diff -Nru a/net/atm/clip.c b/net/atm/clip.c
--- a/net/atm/clip.c	2004-06-22 14:04:02 -07:00
+++ b/net/atm/clip.c	2004-06-22 14:04:02 -07:00
@@ -503,7 +503,7 @@
 	skb_queue_head_init(&copy);
 	skb_migrate(&vcc->sk->sk_receive_queue, &copy);
 	/* re-process everything received between connection setup and MKIP */
-	while ((skb = skb_dequeue(&copy)))
+	while ((skb = skb_dequeue(&copy)) != NULL)
 		if (!clip_devs) {
 			atm_return(vcc,skb->truesize);
 			kfree_skb(skb);
diff -Nru a/net/atm/common.c b/net/atm/common.c
--- a/net/atm/common.c	2004-06-22 14:04:02 -07:00
+++ b/net/atm/common.c	2004-06-22 14:04:02 -07:00
@@ -187,7 +187,7 @@
 
 		vcc_remove_socket(sk);	/* no more receive */
 
-		while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue))) {
+		while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue)) != NULL) {
 			atm_return(vcc,skb->truesize);
 			kfree_skb(skb);
 		}
diff -Nru a/net/atm/lec.c b/net/atm/lec.c
--- a/net/atm/lec.c	2004-06-22 14:04:02 -07:00
+++ b/net/atm/lec.c	2004-06-22 14:04:02 -07:00
@@ -567,7 +567,7 @@
         if (skb_peek(&vcc->sk->sk_receive_queue))
 		printk("%s lec_atm_close: closing with messages pending\n",
                        dev->name);
-        while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue))) {
+        while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue)) != NULL) {
                 atm_return(vcc, skb->truesize);
 		dev_kfree_skb(skb);
         }
@@ -1940,7 +1940,7 @@
                                            priv->path_switching_delay)) {
 			                        struct sk_buff *skb;
 
- 				                while ((skb = skb_dequeue(&entry->tx_wait)))
+ 				                while ((skb = skb_dequeue(&entry->tx_wait)) != NULL)
 					                lec_send(entry->vcc, skb, entry->priv);
                                                 entry->last_used = jiffies;
                                                 entry->status = 
@@ -2337,7 +2337,7 @@
                             entry->status == ESI_FLUSH_PENDING) {
 			        struct sk_buff *skb;
 
- 				while ((skb = skb_dequeue(&entry->tx_wait)))
+ 				while ((skb = skb_dequeue(&entry->tx_wait)) != NULL)
 					lec_send(entry->vcc, skb, entry->priv);
                                 entry->status = ESI_FORWARD_DIRECT;
                                 DPRINTK("LEC_ARP: Flushed\n");
diff -Nru a/net/atm/svc.c b/net/atm/svc.c
--- a/net/atm/svc.c	2004-06-22 14:04:02 -07:00
+++ b/net/atm/svc.c	2004-06-22 14:04:02 -07:00
@@ -66,7 +66,7 @@
 	}
 	/* beware - socket is still in use by atmsigd until the last
 	   as_indicate has been answered */
-	while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue))) {
+	while ((skb = skb_dequeue(&vcc->sk->sk_receive_queue)) != NULL) {
 		DPRINTK("LISTEN REL\n");
 		sigd_enq2(NULL,as_reject,vcc,NULL,NULL,&vcc->qos,0);
 		dev_kfree_skb(skb);

Re: [PATCH][ATM]: fix sparse checker warnings

[David S. Miller]

On Sat, 26 Jun 2004 15:52:30 -0700
Chris Wedgwood <cw@f00f.org> wrote:

> > +++ b/net/atm/br2684.c	2004-06-22 14:04:02 -07:00
> > @@ -562,7 +562,7 @@
> >  	atmvcc->push = br2684_push;
> >  	skb_queue_head_init(&copy);
> >  	skb_migrate(&atmvcc->sk->sk_receive_queue, &copy);
> 
> > - -	while ((skb = skb_dequeue(&copy))) {
> > +	while ((skb = skb_dequeue(&copy)) != NULL) {
> 
> I know it's a matter of style, but I really hate the 'assignment in
> conditional' warning sparse spews out, especially when many people,
> myself included really do write while ((a = b)) --- the extra
> parentheses as a compromise to keep gcc quiet.

I think warning for the ((a=b)) case is annoying but not annoying
enough to fight against it :)

Re: [PATCH][ATM]: fix sparse checker warnings

[David S. Miller]

On Sat, 26 Jun 2004 19:04:42 -0400
"chas williams (contractor)" <chas@cmf.nrl.navy.mil> wrote:

> In message <20040626225230.GA12698@taniwha.stupidest.org>,Chris Wedgwood writes
> :
> >forwarding mangled the patch
> 
> a bit of brillant work on my part.  here it is again but undamaged.
> 
> [PATCH][ATM]: fix sparse checker warnings (by Stephen Hemminger <shemminger@osdl.org>)

Applied, thanks.

CodingStyle: while ((a=b))

[Chris Wedgwood]

On Sat, Jun 26, 2004 at 04:26:41PM -0700, David S. Miller wrote:

> I think warning for the ((a=b)) case is annoying but not annoying
> enough to fight against it :)

Whatever the case, even if I don't like it I would rather see it
consistent.

Maybe Linus can comment as to whether or not we officially
(Documentation/CodingStyle update) want to change things like:

    while ((a=b)) to while ((a=b) != 0)

and similar throughout the code?


  --cw

Re: CodingStyle: while ((a=b))

[chas williams (contractor)]

In message <20040626234221.GB12761@taniwha.stupidest.org>,Chris Wedgwood writes
:
>    while ((a=b)) to while ((a=b) != 0)

adding the != does not make the conditional clearer to me.  in fact
it just make it harder to read.  however, it probably not a good
idea to do assignments in places where someone might not expect to
see them.  the alternative something like this:

	while (1) {
		a = b;
		if (a == 0)
			break;
	}

but its not very pretty either.

Re: CodingStyle: while ((a=b))

[Chris Wedgwood]

On Sat, Jun 26, 2004 at 09:20:20PM -0400, chas williams (contractor) wrote:

> adding the != does not make the conditional clearer to me.  in fact
> it just make it harder to read.

so how about we stop merging these particular 'sparse cleanups' until
there is a consensus on how things should be done?

I'm no saying everyone should agree with me (even though you should)
just that consistency is good.

> however, it probably not a good idea to do assignments in places
> where someone might not expect to see them.

I'm not sure this has ever been a problem in the past has it?

Consider something like:

    while(*a++ = *b++);

I don't see how that's vague or ugly, and rewriting is just making it
worse IMO.


  --cw

Re: CodingStyle: while ((a=b))

[chas williams (contractor)]

In message <20040627012504.GB14444@taniwha.stupidest.org>,Chris Wedgwood writes:
>I'm not sure this has ever been a problem in the past has it?

as a counter example, tired eyes (and the compiler!) cant tell
the difference between:

	while ((a = b))
	if ((a = b))

and

	while ((a == b))
	if ((a == b))

so i just tend to shy away from assignments in conditions.

Re: [PATCH][ATM]: fix sparse checker warnings

[Jeff Garzik]

On Sat, Jun 26, 2004 at 04:26:41PM -0700, David S. Miller wrote:
> On Sat, 26 Jun 2004 15:52:30 -0700
> Chris Wedgwood <cw@f00f.org> wrote:
> 
> > > +++ b/net/atm/br2684.c	2004-06-22 14:04:02 -07:00
> > > @@ -562,7 +562,7 @@
> > >  	atmvcc->push = br2684_push;
> > >  	skb_queue_head_init(&copy);
> > >  	skb_migrate(&atmvcc->sk->sk_receive_queue, &copy);
> > 
> > > - -	while ((skb = skb_dequeue(&copy))) {
> > > +	while ((skb = skb_dequeue(&copy)) != NULL) {
> > 
> > I know it's a matter of style, but I really hate the 'assignment in
> > conditional' warning sparse spews out, especially when many people,
> > myself included really do write while ((a = b)) --- the extra
> > parentheses as a compromise to keep gcc quiet.
> 
> I think warning for the ((a=b)) case is annoying but not annoying
> enough to fight against it :)

FWIW I take a tangential position on the issue:

it causes a fuckton of code churn