* [PATCH] net: Fix wrong sizeof
@ 2009-10-02 9:30 Jean Delvare
2009-10-02 15:14 ` Randy Dunlap
2009-10-02 16:54 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Jean Delvare @ 2009-10-02 9:30 UTC (permalink / raw)
To: LKML, netdev; +Cc: linux-doc, Randy Dunlap, stable
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(-)
--- 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: Fix wrong sizeof
2009-10-02 9:30 [PATCH] net: Fix wrong sizeof Jean Delvare
@ 2009-10-02 15:14 ` Randy Dunlap
2009-10-02 16:54 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Randy Dunlap @ 2009-10-02 15:14 UTC (permalink / raw)
To: Jean Delvare; +Cc: LKML, netdev, linux-doc, stable
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: Fix wrong sizeof
2009-10-02 9:30 [PATCH] net: Fix wrong sizeof Jean Delvare
2009-10-02 15:14 ` Randy Dunlap
@ 2009-10-02 16:54 ` David Miller
[not found] ` <4AC77003.9090805@computer.org>
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2009-10-02 16:54 UTC (permalink / raw)
To: khali; +Cc: linux-kernel, netdev, linux-doc, rdunlap, stable
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] scripts/checkpatch.pl: Add WARN on sizeof(&
[not found] ` <4AC77003.9090805@computer.org>
@ 2009-10-04 1:12 ` Joe Perches
2009-10-05 7:59 ` Américo Wang
2009-10-26 11:17 ` Andy Whitcroft
0 siblings, 2 replies; 6+ messages in thread
From: Joe Perches @ 2009-10-04 1:12 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Jan Ceuleers, David Miller, LKML
On Sat, 2009-10-03 at 17:38 +0200, 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.
Perhaps a checkpatch test.
---
sizeof(&foo) is frequently an error. Warn on its use.
Signed-off-by: Joe Perches <joe@perches.com>
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 87bbb8b..596dd72 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2534,6 +2534,11 @@ sub process {
WARN("plain inline is preferred over $1\n" . $herecurr);
}
+# check for sizeof(&)
+ if ($line =~ /\bsizeof\s*\(\s*\&/) {
+ WARN("sizeof(& should be avoided\n" . $herecurr);
+ }
+
# check for new externs in .c files.
if ($realfile =~ /\.c$/ && defined $stat &&
$stat =~ /^.\s*(?:extern\s+)?$Type\s+($Ident)(\s*)\(/s)
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] scripts/checkpatch.pl: Add WARN on sizeof(&
2009-10-04 1:12 ` [PATCH] scripts/checkpatch.pl: Add WARN on sizeof(& Joe Perches
@ 2009-10-05 7:59 ` Américo Wang
2009-10-26 11:17 ` Andy Whitcroft
1 sibling, 0 replies; 6+ messages in thread
From: Américo Wang @ 2009-10-05 7:59 UTC (permalink / raw)
To: Joe Perches; +Cc: Andy Whitcroft, Jan Ceuleers, David Miller, LKML
On Sun, Oct 4, 2009 at 9:12 AM, Joe Perches <joe@perches.com> wrote:
> On Sat, 2009-10-03 at 17:38 +0200, 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.
>
> Perhaps a checkpatch test.
A quick grep shows we have more:
grep -e '\bsizeof[:space:]*([:space:]*\&.*)' -nr ./
./drivers/media/video/em28xx/em28xx-cards.c:2237:
memset(&dev->info, 0, sizeof(&dev->info));
./drivers/media/video/saa7164/saa7164-cmd.c:350:
memset(&response_t, 0, sizeof(&response_t));
./drivers/media/dvb/dvb-usb/ce6230.c:108: memset(&req, 0, sizeof(&req));
./drivers/video/backlight/lcd.c:59: memset(&ld->fb_notif, 0,
sizeof(&ld->fb_notif));
>
> ---
>
> sizeof(&foo) is frequently an error. Warn on its use.
>
> Signed-off-by: Joe Perches <joe@perches.com>
This patch looks fine for me,
Acked-by: WANG Cong <xiyou.wangcong@gmail.com>
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 87bbb8b..596dd72 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2534,6 +2534,11 @@ sub process {
> WARN("plain inline is preferred over $1\n" . $herecurr);
> }
>
> +# check for sizeof(&)
> + if ($line =~ /\bsizeof\s*\(\s*\&/) {
> + WARN("sizeof(& should be avoided\n" . $herecurr);
> + }
> +
> # check for new externs in .c files.
> if ($realfile =~ /\.c$/ && defined $stat &&
> $stat =~ /^.\s*(?:extern\s+)?$Type\s+($Ident)(\s*)\(/s)
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] scripts/checkpatch.pl: Add WARN on sizeof(&
2009-10-04 1:12 ` [PATCH] scripts/checkpatch.pl: Add WARN on sizeof(& Joe Perches
2009-10-05 7:59 ` Américo Wang
@ 2009-10-26 11:17 ` Andy Whitcroft
1 sibling, 0 replies; 6+ messages in thread
From: Andy Whitcroft @ 2009-10-26 11:17 UTC (permalink / raw)
To: Joe Perches; +Cc: Jan Ceuleers, David Miller, LKML
On Sat, Oct 03, 2009 at 06:12:49PM -0700, Joe Perches wrote:
> On Sat, 2009-10-03 at 17:38 +0200, 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.
>
> Perhaps a checkpatch test.
>
> ---
>
> sizeof(&foo) is frequently an error. Warn on its use.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Seems reasonable. A slightly modified version of this is now in my
tree. Thanks!
http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/checkpatch.pl-testing
-apw
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-10-26 11:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-02 9:30 [PATCH] net: Fix wrong sizeof Jean Delvare
2009-10-02 15:14 ` Randy Dunlap
2009-10-02 16:54 ` David Miller
[not found] ` <4AC77003.9090805@computer.org>
2009-10-04 1:12 ` [PATCH] scripts/checkpatch.pl: Add WARN on sizeof(& Joe Perches
2009-10-05 7:59 ` Américo Wang
2009-10-26 11:17 ` Andy Whitcroft
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox