diff mbox

net: Fix wrong sizeof

Message ID 20091002113038.1dc3d284@hyperion.delvare
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Jean Delvare Oct. 2, 2009, 9:30 a.m. UTC
Which is why I have always preferred sizeof(struct foo) over
sizeof(var).

Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Randy Dunlap <rdunlap@xenotime.net>
---
Stable team, the non-documentation part of this fix applies to 2.6.31,
2.6.30 and 2.6.27.

 Documentation/networking/timestamping/timestamping.c |    2 +-
 drivers/net/iseries_veth.c                           |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Randy.Dunlap Oct. 2, 2009, 3:14 p.m. UTC | #1
On Fri, 2 Oct 2009 11:30:38 +0200 Jean Delvare wrote:

> Which is why I have always preferred sizeof(struct foo) over
> sizeof(var).
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Randy Dunlap <rdunlap@xenotime.net>

Acked-by: Randy Dunlap <rdunlap@xenotime.net>

I also prefer to use sizeof(struct xyz) in my non-kernel code
instead of sizeof(var).

> ---
> Stable team, the non-documentation part of this fix applies to 2.6.31,
> 2.6.30 and 2.6.27.
> 
>  Documentation/networking/timestamping/timestamping.c |    2 +-
>  drivers/net/iseries_veth.c                           |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> --- linux-2.6.32-rc1.orig/Documentation/networking/timestamping/timestamping.c	2009-06-10 05:05:27.000000000 +0200
> +++ linux-2.6.32-rc1/Documentation/networking/timestamping/timestamping.c	2009-10-02 11:07:19.000000000 +0200
> @@ -381,7 +381,7 @@ int main(int argc, char **argv)
>  	memset(&hwtstamp, 0, sizeof(hwtstamp));
>  	strncpy(hwtstamp.ifr_name, interface, sizeof(hwtstamp.ifr_name));
>  	hwtstamp.ifr_data = (void *)&hwconfig;
> -	memset(&hwconfig, 0, sizeof(&hwconfig));
> +	memset(&hwconfig, 0, sizeof(hwconfig));
>  	hwconfig.tx_type =
>  		(so_timestamping_flags & SOF_TIMESTAMPING_TX_HARDWARE) ?
>  		HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
> --- linux-2.6.32-rc1.orig/drivers/net/iseries_veth.c	2009-09-28 10:28:42.000000000 +0200
> +++ linux-2.6.32-rc1/drivers/net/iseries_veth.c	2009-10-02 11:07:15.000000000 +0200
> @@ -495,7 +495,7 @@ static void veth_take_cap_ack(struct vet
>  			   cnx->remote_lp);
>  	} else {
>  		memcpy(&cnx->cap_ack_event, event,
> -		       sizeof(&cnx->cap_ack_event));
> +		       sizeof(cnx->cap_ack_event));
>  		cnx->state |= VETH_STATE_GOTCAPACK;
>  		veth_kick_statemachine(cnx);
>  	}
> 
> 
> -- 
> Jean Delvare
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


---
~Randy
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Oct. 2, 2009, 4:54 p.m. UTC | #2
From: Jean Delvare <khali@linux-fr.org>
Date: Fri, 2 Oct 2009 11:30:38 +0200

> Which is why I have always preferred sizeof(struct foo) over
> sizeof(var).
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Randy Dunlap <rdunlap@xenotime.net>

Any time you see "&" in a sizeof() expression, it's almost
certainly a bug.  Something for the folks with automated
tools to look for if they haven't already :-)

I'll apply this, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Ceuleers Oct. 3, 2009, 3:38 p.m. UTC | #3
David Miller wrote:
> Any time you see "&" in a sizeof() expression, it's almost
> certainly a bug.  Something for the folks with automated
> tools to look for if they haven't already :-)

Your remark prompted me to find four more instances of such bugs (none of which in the networking bits). I have submitted patches.

Thank you.

Jan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Ceuleers Oct. 4, 2009, 10:36 a.m. UTC | #4
Jan Ceuleers wrote:
> David Miller wrote:
>> Any time you see "&" in a sizeof() expression, it's almost
>> certainly a bug.  Something for the folks with automated
>> tools to look for if they haven't already :-)
> 
> Your remark prompted me to find four more instances of such bugs (none of which in the networking bits). I have submitted patches.

As a matter of fact, Jean Delvare had already found all of these and submitted patches.

Jan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- linux-2.6.32-rc1.orig/Documentation/networking/timestamping/timestamping.c	2009-06-10 05:05:27.000000000 +0200
+++ linux-2.6.32-rc1/Documentation/networking/timestamping/timestamping.c	2009-10-02 11:07:19.000000000 +0200
@@ -381,7 +381,7 @@  int main(int argc, char **argv)
 	memset(&hwtstamp, 0, sizeof(hwtstamp));
 	strncpy(hwtstamp.ifr_name, interface, sizeof(hwtstamp.ifr_name));
 	hwtstamp.ifr_data = (void *)&hwconfig;
-	memset(&hwconfig, 0, sizeof(&hwconfig));
+	memset(&hwconfig, 0, sizeof(hwconfig));
 	hwconfig.tx_type =
 		(so_timestamping_flags & SOF_TIMESTAMPING_TX_HARDWARE) ?
 		HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
--- linux-2.6.32-rc1.orig/drivers/net/iseries_veth.c	2009-09-28 10:28:42.000000000 +0200
+++ linux-2.6.32-rc1/drivers/net/iseries_veth.c	2009-10-02 11:07:15.000000000 +0200
@@ -495,7 +495,7 @@  static void veth_take_cap_ack(struct vet
 			   cnx->remote_lp);
 	} else {
 		memcpy(&cnx->cap_ack_event, event,
-		       sizeof(&cnx->cap_ack_event));
+		       sizeof(cnx->cap_ack_event));
 		cnx->state |= VETH_STATE_GOTCAPACK;
 		veth_kick_statemachine(cnx);
 	}