d80211: clean up some list and loop code

d80211: clean up some list and loop code

[Johannes Berg]

Remove things like "for (;;)" or "for (; condition ;)".
Ever heard of while loops?

Also clean up a bunch of things that list.h offers. No need to code it
here.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---

This is just insane. Can't we just rewrite the whole thing? :)

I'm not really sure we need list_for_each_entry_safe in the first hunk
but it doesn't hurt either.

--- wireless-dev.orig/net/d80211/ieee80211_sta.c	2006-11-16 23:44:12.624935990 +0100
+++ wireless-dev/net/d80211/ieee80211_sta.c	2006-11-16 23:44:15.944935990 +0100
@@ -1314,16 +1314,10 @@ void ieee80211_rx_bss_list_init(struct n
 void ieee80211_rx_bss_list_deinit(struct net_device *dev)
 {
 	struct ieee80211_local *local = dev->ieee80211_ptr;
-	struct ieee80211_sta_bss *bss;
-	struct list_head *ptr;
+	struct ieee80211_sta_bss *bss, *tmp;
 
-	for (;;) {
-		ptr = local->sta_bss_list.next;
-		if (!ptr || ptr == &local->sta_bss_list)
-			break;
-		bss = list_entry(ptr, struct ieee80211_sta_bss, list);
+	list_for_each_entry_safe(bss, tmp, &local->sta_bss_list, list)
 		ieee80211_rx_bss_put(dev, bss);
-	}
 }
 
 
--- wireless-dev.orig/net/d80211/sta_info.c	2006-11-16 23:40:48.164935990 +0100
+++ wireless-dev/net/d80211/sta_info.c	2006-11-16 23:55:34.634935990 +0100
@@ -299,7 +299,7 @@ static void sta_info_cleanup_expire_buff
 	if (skb_queue_empty(&sta->ps_tx_buf))
 		return;
 
-	for (;;) {
+	while (1) {
 		spin_lock_irqsave(&sta->ps_tx_buf.lock, flags);
 		skb = skb_peek(&sta->ps_tx_buf);
 		if (sta_info_buffer_expired(local, sta, skb)) {
@@ -324,16 +324,13 @@ static void sta_info_cleanup_expire_buff
 static void sta_info_cleanup(unsigned long data)
 {
 	struct ieee80211_local *local = (struct ieee80211_local *) data;
-	struct list_head *ptr;
+	struct sta_info *sta, *tmp;
 
 	spin_lock_bh(&local->sta_lock);
-	ptr = local->sta_list.next;
-	while (ptr && ptr != &local->sta_list) {
-		struct sta_info *sta = (struct sta_info *) ptr;
+	list_for_each_entry_safe(sta, tmp, &local->sta_list, list) {
 		__sta_info_get(sta);
 		sta_info_cleanup_expire_buffered(local, sta);
 		sta_info_put(sta);
-		ptr = ptr->next;
 	}
 	spin_unlock_bh(&local->sta_lock);
 
@@ -411,14 +408,11 @@ int sta_info_start(struct ieee80211_loca
 
 void sta_info_stop(struct ieee80211_local *local)
 {
-	struct list_head *ptr;
+	struct sta_info *sta, *tmp;
 
 	del_timer(&local->sta_cleanup);
 
-	ptr = local->sta_list.next;
-	while (ptr && ptr != &local->sta_list) {
-		struct sta_info *sta = (struct sta_info *) ptr;
-		ptr = ptr->next;
+	list_for_each_entry_safe(sta, tmp, &local->sta_list, list) {
 		/* sta_info_free must be called with 0 as the last
 		 * parameter to ensure all sysfs sta entries are
 		 * unregistered. We don't need locking at this
--- wireless-dev.orig/net/d80211/wme.c	2006-11-16 23:40:37.134935990 +0100
+++ wireless-dev/net/d80211/wme.c	2006-11-16 23:40:40.764935990 +0100
@@ -211,8 +211,7 @@ static inline int classify80211(struct s
 	skb->priority = classify_1d(skb, qd);
 
 	/* incase we are a client verify acm is not set for this ac */
-	for (; unlikely(local->wmm_acm & BIT(skb->priority)); )
-	{
+	while (unlikely(local->wmm_acm & BIT(skb->priority))) {
 		if (wme_downgrade_ac(skb)) {
 			/* No AC with lower priority has acm=0,
 			* drop packet. */

Re: d80211: clean up some list and loop code

[John W. Linville]

On Fri, Nov 17, 2006 at 12:04:29AM +0100, Johannes Berg wrote:
> Remove things like "for (;;)" or "for (; condition ;)".
> Ever heard of while loops?

> --- wireless-dev.orig/net/d80211/sta_info.c	2006-11-16 23:40:48.164935990 +0100
> +++ wireless-dev/net/d80211/sta_info.c	2006-11-16 23:55:34.634935990 +0100
> @@ -299,7 +299,7 @@ static void sta_info_cleanup_expire_buff
>  	if (skb_queue_empty(&sta->ps_tx_buf))
>  		return;
>  
> -	for (;;) {
> +	while (1) {
>  		spin_lock_irqsave(&sta->ps_tx_buf.lock, flags);
>  		skb = skb_peek(&sta->ps_tx_buf);
>  		if (sta_info_buffer_expired(local, sta, skb)) {

FWIW, I think I prefer the "for (;;)" version for endless loops.
It looks more intentional to me.  Some grep'ing showed nearly equal
usage of "for (;;)" versus "while (1)".  Is there any "official"
preference?  I don't see anything in CodingStyle about it.

I agree with the other cleanups.

John
-- 
John W. Linville
linville@tuxdriver.com

Re: d80211: clean up some list and loop code

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]


> > Ever heard of while loops?

was more about

> "for (; condition ;)".

:)


> FWIW, I think I prefer the "for (;;)" version for endless loops.
> It looks more intentional to me.  Some grep'ing showed nearly equal
> usage of "for (;;)" versus "while (1)".  Is there any "official"
> preference?  I don't see anything in CodingStyle about it.

I think while (1) looks more intentional, but I suppose that's because I
grew up with pascal where you can't do all these weird things with for
loops.

Feel free to drop this part of the patch if you wish, or, if you want, I
can resend without that too :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: d80211: clean up some list and loop code

[John W. Linville]

On Fri, Nov 17, 2006 at 12:37:40AM +0100, Johannes Berg wrote:

> > FWIW, I think I prefer the "for (;;)" version for endless loops.
> > It looks more intentional to me.  Some grep'ing showed nearly equal
> > usage of "for (;;)" versus "while (1)".  Is there any "official"
> > preference?  I don't see anything in CodingStyle about it.
> 
> I think while (1) looks more intentional, but I suppose that's because I
> grew up with pascal where you can't do all these weird things with for
> loops.
> 
> Feel free to drop this part of the patch if you wish, or, if you want, I
> can resend without that too :)

No need to resend.  Any other opinions on this?  I suppose it is just
a beached discussion...

-- 
John W. Linville
linville@tuxdriver.com

Re: d80211: clean up some list and loop code

[John W. Linville]

On Thu, Nov 16, 2006 at 07:00:02PM -0500, John W. Linville wrote:

> No need to resend.  Any other opinions on this?  I suppose it is just
> a beached discussion...

What the heck was I typing?

"a bikeshed discussion"

-- 
John W. Linville
linville@tuxdriver.com

Re: d80211: clean up some list and loop code

[Dave Dillow]

On Thu, 2006-11-16 at 18:25 -0500, John W. Linville wrote:
> On Fri, Nov 17, 2006 at 12:04:29AM +0100, Johannes Berg wrote:
> > Remove things like "for (;;)" or "for (; condition ;)".
> > Ever heard of while loops?
> 
> > --- wireless-dev.orig/net/d80211/sta_info.c	2006-11-16 23:40:48.164935990 +0100
> > +++ wireless-dev/net/d80211/sta_info.c	2006-11-16 23:55:34.634935990 +0100
> > @@ -299,7 +299,7 @@ static void sta_info_cleanup_expire_buff
> >  	if (skb_queue_empty(&sta->ps_tx_buf))
> >  		return;
> >  
> > -	for (;;) {
> > +	while (1) {
> >  		spin_lock_irqsave(&sta->ps_tx_buf.lock, flags);
> >  		skb = skb_peek(&sta->ps_tx_buf);
> >  		if (sta_info_buffer_expired(local, sta, skb)) {
> 
> FWIW, I think I prefer the "for (;;)" version for endless loops.
> It looks more intentional to me.  Some grep'ing showed nearly equal
> usage of "for (;;)" versus "while (1)".  Is there any "official"
> preference?  I don't see anything in CodingStyle about it.

I don't know about official preference, but in ages past, using
"for(;;)" was preferable because "while(1)" would often invoke a
"statement has no effect" warning from the compiler. gcc 4.0.2 doesn't
have this problem, but IIRC it used to.

FWIW, I prefer "for(;;)" because of painful memories with other people's
code and -Werror, but I don't barf at "while(1)".

Re: d80211: clean up some list and loop code

[Jouni Malinen]

On Fri, Nov 17, 2006 at 12:04:29AM +0100, Johannes Berg wrote:
> Remove things like "for (;;)" or "for (; condition ;)".
> Ever heard of while loops?

Isn't "for (;;)" the standard way of doing infinite loops? Please don't
change that. Changing loops to use list_for_each*() is of course
welcome.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: d80211: clean up some list and loop code

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 348 bytes --]

On Thu, 2006-11-16 at 15:40 -0800, Jouni Malinen wrote:

> Isn't "for (;;)" the standard way of doing infinite loops? Please don't
> change that.

In *BSD it is apparently mandated, but Linux doesn't seem to do that as
John pointed out. Personally, I find while (1) much easier to read but
really, there's one occurrence so ...

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]