[patch] devinet: cleanup if statements

[patch] devinet: cleanup if statements

[pmeda]

Cleanup the devinet if statements.
 - when there is no colon, interface name is same as device.
 - ifa_label is an array, not a pointer, and so can never be null.

Signed-Off-by: Prasanna Meda <pmeda@akamai.com>

--- a/net/ipv4/devinet.c	Wed Jun  1 23:54:37 2005
+++ b/net/ipv4/devinet.c	Wed Jun  1 23:57:16 2005
@@ -636,10 +636,7 @@
 			ret = -ENOBUFS;
 			if ((ifa = inet_alloc_ifa()) == NULL)
 				break;
-			if (colon)
-				memcpy(ifa->ifa_label, ifr.ifr_name, IFNAMSIZ);
-			else
-				memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
+			memcpy(ifa->ifa_label, ifr.ifr_name, IFNAMSIZ);
 		} else {
 			ret = 0;
 			if (ifa->ifa_local == sin->sin_addr.s_addr)
@@ -746,10 +743,7 @@
 		if (len < (int) sizeof(ifr))
 			break;
 		memset(&ifr, 0, sizeof(struct ifreq));
-		if (ifa->ifa_label)
-			strcpy(ifr.ifr_name, ifa->ifa_label);
-		else
-			strcpy(ifr.ifr_name, dev->name);
+		strcpy(ifr.ifr_name, ifa->ifa_label);
 
 		(*(struct sockaddr_in *)&ifr.ifr_addr).sin_family = AF_INET;
 		(*(struct sockaddr_in *)&ifr.ifr_addr).sin_addr.s_addr =

Re: [patch] devinet: cleanup if statements

[David S. Miller]

From: pmeda@akamai.com
Date: Tue, 7 Jun 2005 13:32:44 -0700

> Cleanup the devinet if statements.
>  - when there is no colon, interface name is same as device.
>  - ifa_label is an array, not a pointer, and so can never be null.
> 
> Signed-Off-by: Prasanna Meda <pmeda@akamai.com>

Ok, I can see how your first change is correct.

When there is a colon, we've modified ifr.ifr_name by
patching the ':' character to be a '\0'.  This is for
the __dev_get_by_name() lookup.

After that lookup, we re-patch the ':' character back
into ifr.ifr_name.  So indeed, always using the ifr_name
in that code block would be correct.

The second hunk of your patch seems to defeat the intention
of that code.  I believe the idea is that if the label and
the device name differ, use the label.

This whole area is pretty messy, we should examine the true
intended semantics of the ifa_label stuff.

Re: [patch] devinet: cleanup if statements

[Andi Kleen]

"David S. Miller" <davem@davemloft.net> writes:
> 
> This whole area is pretty messy, we should examine the true
> intended semantics of the ifa_label stuff.

Perhaps it would be best to just retire that 2.0 alias compatibility 
cruft now. Everybody should be using ifconfig or iproute2 by now
that can add or remove addresses directly without the compatibility
syntax.

They never worked fully in corner cases anyways, e.g. SIOCSIFNAME with
these aliases was always a problem.

-Andi

Re: [patch] devinet: cleanup if statements

[Herbert Xu]

David S. Miller <davem@davemloft.net> wrote:
> 
> The second hunk of your patch seems to defeat the intention
> of that code.  I believe the idea is that if the label and
> the device name differ, use the label.

Actually I think Prasanna is right.  The if conditional is testing
whether ifa->ifa_label is NULL.  As ifa->ifa_label is an array and
it's not the first element in the structure, it can't possibly be
NULL.

With your interpretation above his patch is correct as well.  If
we want to use the label when it is different from the device name,
then it is equivalent to always use the label since the only time
we'd use the device name is when it's equal to the label :)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [patch] devinet: cleanup if statements

[Prasanna Meda]

Herbert Xu wrote:

>
>
> With your interpretation above his patch is correct as well.  If
> we want to use the label when it is different from the device name,
> then it is equivalent to always use the label since the only time
> we'd use the device name is when it's equal to the label :)

Sounds correct to me. Actually I did not think about second
interpretation in first.

Thanks,
Prasanna.