[PATCH] Eliminate some unnecessary gotos in tcp v4 hash handling

[PATCH] Eliminate some unnecessary gotos in tcp v4 hash handling

[Andi Kleen]

The compiler eliminates them anyways and this makes the code easier to read
and shorter.

Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux-2.6.21-rc3-net/net/ipv4/tcp_ipv4.c
===================================================================
--- linux-2.6.21-rc3-net.orig/net/ipv4/tcp_ipv4.c
+++ linux-2.6.21-rc3-net/net/ipv4/tcp_ipv4.c
@@ -2052,8 +2052,7 @@ static void *established_get_first(struc
 			if (sk->sk_family != st->family) {
 				continue;
 			}
-			rc = sk;
-			goto out;
+			return sk;
 		}
 		st->state = TCP_SEQ_STATE_TIME_WAIT;
 		inet_twsk_for_each(tw, node,
@@ -2061,13 +2060,11 @@ static void *established_get_first(struc
 			if (tw->tw_family != st->family) {
 				continue;
 			}
-			rc = tw;
-			goto out;
+			return tw;
 		}
 		read_unlock(&tcp_hashinfo.ehash[st->bucket].lock);
 		st->state = TCP_SEQ_STATE_ESTABLISHED;
 	}
-out:
 	return rc;
 }
 
@@ -2088,10 +2085,8 @@ get_tw:
 		while (tw && tw->tw_family != st->family) {
 			tw = tw_next(tw);
 		}
-		if (tw) {
-			cur = tw;
-			goto out;
-		}
+		if (tw)
+			return tw; 
 		read_unlock(&tcp_hashinfo.ehash[st->bucket].lock);
 		st->state = TCP_SEQ_STATE_ESTABLISHED;
 
@@ -2111,16 +2106,12 @@ get_tw:
 
 	sk_for_each_from(sk, node) {
 		if (sk->sk_family == st->family)
-			goto found;
+			return sk; 
 	}
 
 	st->state = TCP_SEQ_STATE_TIME_WAIT;
 	tw = tw_head(&tcp_hashinfo.ehash[st->bucket].twchain);
 	goto get_tw;
-found:
-	cur = sk;
-out:
-	return cur;
 }
 
 static void *established_get_idx(struct seq_file *seq, loff_t pos)

Re: [PATCH] Eliminate some unnecessary gotos in tcp v4 hash handling

[David Miller]

From: Andi Kleen <ak@suse.de>
Date: Thu, 5 Apr 2007 12:48:44 +0200

> The compiler eliminates them anyways and this makes the code easier to read
> and shorter.
> 
> Signed-off-by: Andi Kleen <ak@suse.de>

It depends upon who you ask :-)

For someone trying to verify "all code paths lead to foo and
therefore release resources and drop locks" it's always better
to use a goto to some common unwind handler at the end of
the function, regardless of what the compiler does with it.

When you're doing such an audit, return statements in the middle
of the function are red flags and make the audit more difficult.
:-)

But in these cases it's just straight returns and there are no
resources to release so I'll apply this patch, thanks!

Re: [PATCH] Eliminate some unnecessary gotos in tcp v4 hash handling

[David Miller]

From: Andi Kleen <ak@suse.de>
Date: Thu, 5 Apr 2007 12:48:44 +0200

> 
> The compiler eliminates them anyways and this makes the code easier to read
> and shorter.
> 
> Signed-off-by: Andi Kleen <ak@suse.de>

Actually I won't apply this patch, ever, -ENOCOMPILE.

Andi, please spare me with these patches if you won't even bother
trying to compile them.  It doesn't matter how easy to read the code
is if it doesn't build. :-/

I found this build failure just by reviewing your patch for
correctness, I didn't even need to give it to the compiler to see that
you left the "cur = NULL; goto out;" case in there yet removed the
"out" label.

Don't bother posting the fixed version, I'll ignore it.  You just
basically proved how pointless this patch is, and how little you
yourself even care about it.