* [PATCH] gianfar: Fix possible overrun and simplify interrupt name field creation
@ 2012-03-17 19:05 Joe Perches
2012-03-22 2:34 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2012-03-17 19:05 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: Sandeep Gopalpet
Space allocated for int_name_<foo> is unsufficient for
maximal device name, expand it.
Code to create int_name_<foo> is obscure, simplify it
by using sprintf.
Found by looking for unnecessary \ line continuations.
Uncompiled, untested.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/ethernet/freescale/gianfar.c | 39 +++++------------------------
drivers/net/ethernet/freescale/gianfar.h | 2 +-
2 files changed, 8 insertions(+), 33 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index adb0ae4..770b8cf 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -971,7 +971,6 @@ static int gfar_probe(struct platform_device *ofdev)
struct gfar_private *priv = NULL;
struct gfar __iomem *regs = NULL;
int err = 0, i, grp_idx = 0;
- int len_devname;
u32 rstat = 0, tstat = 0, rqueue = 0, tqueue = 0;
u32 isrg = 0;
u32 __iomem *baddr;
@@ -1172,40 +1171,16 @@ static int gfar_probe(struct platform_device *ofdev)
priv->device_flags & FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);
/* fill out IRQ number and name fields */
- len_devname = strlen(dev->name);
for (i = 0; i < priv->num_grps; i++) {
- strncpy(&priv->gfargrp[i].int_name_tx[0], dev->name,
- len_devname);
if (priv->device_flags & FSL_GIANFAR_DEV_HAS_MULTI_INTR) {
- strncpy(&priv->gfargrp[i].int_name_tx[len_devname],
- "_g", sizeof("_g"));
- priv->gfargrp[i].int_name_tx[
- strlen(priv->gfargrp[i].int_name_tx)] = i+48;
- strncpy(&priv->gfargrp[i].int_name_tx[strlen(
- priv->gfargrp[i].int_name_tx)],
- "_tx", sizeof("_tx") + 1);
-
- strncpy(&priv->gfargrp[i].int_name_rx[0], dev->name,
- len_devname);
- strncpy(&priv->gfargrp[i].int_name_rx[len_devname],
- "_g", sizeof("_g"));
- priv->gfargrp[i].int_name_rx[
- strlen(priv->gfargrp[i].int_name_rx)] = i+48;
- strncpy(&priv->gfargrp[i].int_name_rx[strlen(
- priv->gfargrp[i].int_name_rx)],
- "_rx", sizeof("_rx") + 1);
-
- strncpy(&priv->gfargrp[i].int_name_er[0], dev->name,
- len_devname);
- strncpy(&priv->gfargrp[i].int_name_er[len_devname],
- "_g", sizeof("_g"));
- priv->gfargrp[i].int_name_er[strlen(
- priv->gfargrp[i].int_name_er)] = i+48;
- strncpy(&priv->gfargrp[i].int_name_er[strlen(\
- priv->gfargrp[i].int_name_er)],
- "_er", sizeof("_er") + 1);
+ sprintf(priv->gfargrp[i].int_name_tx, "%s%s%c%s",
+ dev->name, "_g", '0' + i, "_tx");
+ sprintf(priv->gfargrp[i].int_name_rx, "%s%s%c%s",
+ dev->name, "_g", '0' + i, "_rx");
+ sprintf(priv->gfargrp[i].int_name_er, "%s%s%c%s",
+ dev->name, "_g", '0' + i, "_er");
} else
- priv->gfargrp[i].int_name_tx[len_devname] = '\0';
+ strcpy(priv->gfargrp[i].int_name_tx, dev->name);
}
/* Initialize the filer table */
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 4fe0f34..e537d81 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -520,7 +520,7 @@ extern const char gfar_driver_version[];
#define RXFCB_PERR_MASK 0x000c
#define RXFCB_PERR_BADL3 0x0008
-#define GFAR_INT_NAME_MAX IFNAMSIZ + 4
+#define GFAR_INT_NAME_MAX (IFNAMSIZ + 6) /* '_g#_xx' */
struct txbd8
{
--
1.7.8.111.gad25c.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] gianfar: Fix possible overrun and simplify interrupt name field creation
2012-03-17 19:05 [PATCH] gianfar: Fix possible overrun and simplify interrupt name field creation Joe Perches
@ 2012-03-22 2:34 ` David Miller
2012-03-23 3:34 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2012-03-22 2:34 UTC (permalink / raw)
To: joe; +Cc: netdev, linux-kernel, sandeep.kumar
From: Joe Perches <joe@perches.com>
Date: Sat, 17 Mar 2012 12:05:38 -0700
> Space allocated for int_name_<foo> is unsufficient for
> maximal device name, expand it.
>
> Code to create int_name_<foo> is obscure, simplify it
> by using sprintf.
>
> Found by looking for unnecessary \ line continuations.
> Uncompiled, untested.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Can a gianfar expert please review and test this patch?
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gianfar: Fix possible overrun and simplify interrupt name field creation
2012-03-22 2:34 ` David Miller
@ 2012-03-23 3:34 ` David Miller
2012-03-23 23:37 ` Paul Gortmaker
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2012-03-23 3:34 UTC (permalink / raw)
To: joe; +Cc: netdev, linux-kernel, sandeep.kumar
From: David Miller <davem@davemloft.net>
Date: Wed, 21 Mar 2012 22:34:57 -0400 (EDT)
> From: Joe Perches <joe@perches.com>
> Date: Sat, 17 Mar 2012 12:05:38 -0700
>
>> Space allocated for int_name_<foo> is unsufficient for
>> maximal device name, expand it.
>>
>> Code to create int_name_<foo> is obscure, simplify it
>> by using sprintf.
>>
>> Found by looking for unnecessary \ line continuations.
>> Uncompiled, untested.
>>
>> Signed-off-by: Joe Perches <joe@perches.com>
>
> Can a gianfar expert please review and test this patch?
If you gianfar guys don't show a sign of life in the next 24 hours I'm
just going to apply Joe's patch as-is. I refuse to just let it rot in
patchwork like this.
And then you don't get to complain if his change breaks your driver
because I asked multiple times for testing and/or review.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gianfar: Fix possible overrun and simplify interrupt name field creation
2012-03-23 3:34 ` David Miller
@ 2012-03-23 23:37 ` Paul Gortmaker
2012-03-25 17:10 ` [PATCH v2] " Paul Gortmaker
0 siblings, 1 reply; 6+ messages in thread
From: Paul Gortmaker @ 2012-03-23 23:37 UTC (permalink / raw)
To: David Miller; +Cc: joe, netdev, linux-kernel, sandeep.kumar
On Thu, Mar 22, 2012 at 11:34 PM, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 21 Mar 2012 22:34:57 -0400 (EDT)
>
>> From: Joe Perches <joe@perches.com>
>> Date: Sat, 17 Mar 2012 12:05:38 -0700
>>
>>> Space allocated for int_name_<foo> is unsufficient for
>>> maximal device name, expand it.
>>>
>>> Code to create int_name_<foo> is obscure, simplify it
>>> by using sprintf.
>>>
>>> Found by looking for unnecessary \ line continuations.
>>> Uncompiled, untested.
>>>
>>> Signed-off-by: Joe Perches <joe@perches.com>
>>
>> Can a gianfar expert please review and test this patch?
>
> If you gianfar guys don't show a sign of life in the next 24 hours I'm
> just going to apply Joe's patch as-is. I refuse to just let it rot in
> patchwork like this.
>
> And then you don't get to complain if his change breaks your driver
> because I asked multiple times for testing and/or review.
I don't claim to be a giant fart expert, but the sbc8641D has 4 of
the gianfar ports, and so I can use that hardware to look into
testing this on a real board and check that a cat /proc/interrupts
doesn't do something evil this weekend. The patch looks sane in
that it culls lines of open coded junk... FWIW.
Paul.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] gianfar: Fix possible overrun and simplify interrupt name field creation
2012-03-23 23:37 ` Paul Gortmaker
@ 2012-03-25 17:10 ` Paul Gortmaker
2012-03-25 17:29 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Paul Gortmaker @ 2012-03-25 17:10 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, afleming, Joe Perches, Paul Gortmaker
From: Joe Perches <joe@perches.com>
Space allocated for int_name_<foo> is insufficient for
maximal device name, expand it.
Code to create int_name_<foo> is obscure, simplify it
by using sprintf.
Found by looking for unnecessary \ line continuations.
Signed-off-by: Joe Perches <joe@perches.com>
Tested-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
[v2: delete "uncompiled/untested" and fix typo in commit log]
The IRQ names appear unchanged after applying this patch:
root@SBC8641D:/root> for i in `seq 3` ; do ifconfig eth$i 192.168.0.$i ; done
root@SBC8641D:/root> cat /proc/interrupts
CPU0 CPU1
16: 0 0 MPIC Level phy_interrupt, phy_interrupt, phy_interrupt, phy_interrupt
29: 6884 0 MPIC Level eth0_g0_tx
30: 0 11578 MPIC Level eth0_g0_rx
31: 0 0 MPIC Level eth2_g0_tx
32: 0 0 MPIC Level eth2_g0_rx
33: 0 0 MPIC Level eth2_g0_er
34: 0 0 MPIC Level eth0_g0_er
35: 0 0 MPIC Level eth1_g0_tx
36: 0 0 MPIC Level eth1_g0_rx
37: 0 0 MPIC Level eth3_g0_tx
38: 0 0 MPIC Level eth3_g0_rx
39: 0 0 MPIC Level eth3_g0_er
40: 0 0 MPIC Level eth1_g0_er
Having them all in group "g0" may seem odd, but it is definitely
the same as it was before the commit. Tested on mar23 linux-next.
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index d9428f0..e7bed53 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -968,7 +968,6 @@ static int gfar_probe(struct platform_device *ofdev)
struct gfar_private *priv = NULL;
struct gfar __iomem *regs = NULL;
int err = 0, i, grp_idx = 0;
- int len_devname;
u32 rstat = 0, tstat = 0, rqueue = 0, tqueue = 0;
u32 isrg = 0;
u32 __iomem *baddr;
@@ -1169,40 +1168,16 @@ static int gfar_probe(struct platform_device *ofdev)
priv->device_flags & FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);
/* fill out IRQ number and name fields */
- len_devname = strlen(dev->name);
for (i = 0; i < priv->num_grps; i++) {
- strncpy(&priv->gfargrp[i].int_name_tx[0], dev->name,
- len_devname);
if (priv->device_flags & FSL_GIANFAR_DEV_HAS_MULTI_INTR) {
- strncpy(&priv->gfargrp[i].int_name_tx[len_devname],
- "_g", sizeof("_g"));
- priv->gfargrp[i].int_name_tx[
- strlen(priv->gfargrp[i].int_name_tx)] = i+48;
- strncpy(&priv->gfargrp[i].int_name_tx[strlen(
- priv->gfargrp[i].int_name_tx)],
- "_tx", sizeof("_tx") + 1);
-
- strncpy(&priv->gfargrp[i].int_name_rx[0], dev->name,
- len_devname);
- strncpy(&priv->gfargrp[i].int_name_rx[len_devname],
- "_g", sizeof("_g"));
- priv->gfargrp[i].int_name_rx[
- strlen(priv->gfargrp[i].int_name_rx)] = i+48;
- strncpy(&priv->gfargrp[i].int_name_rx[strlen(
- priv->gfargrp[i].int_name_rx)],
- "_rx", sizeof("_rx") + 1);
-
- strncpy(&priv->gfargrp[i].int_name_er[0], dev->name,
- len_devname);
- strncpy(&priv->gfargrp[i].int_name_er[len_devname],
- "_g", sizeof("_g"));
- priv->gfargrp[i].int_name_er[strlen(
- priv->gfargrp[i].int_name_er)] = i+48;
- strncpy(&priv->gfargrp[i].int_name_er[strlen(\
- priv->gfargrp[i].int_name_er)],
- "_er", sizeof("_er") + 1);
+ sprintf(priv->gfargrp[i].int_name_tx, "%s%s%c%s",
+ dev->name, "_g", '0' + i, "_tx");
+ sprintf(priv->gfargrp[i].int_name_rx, "%s%s%c%s",
+ dev->name, "_g", '0' + i, "_rx");
+ sprintf(priv->gfargrp[i].int_name_er, "%s%s%c%s",
+ dev->name, "_g", '0' + i, "_er");
} else
- priv->gfargrp[i].int_name_tx[len_devname] = '\0';
+ strcpy(priv->gfargrp[i].int_name_tx, dev->name);
}
/* Initialize the filer table */
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index fc2488a..4c9f8d4 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -517,7 +517,7 @@ extern const char gfar_driver_version[];
#define RXFCB_PERR_MASK 0x000c
#define RXFCB_PERR_BADL3 0x0008
-#define GFAR_INT_NAME_MAX IFNAMSIZ + 4
+#define GFAR_INT_NAME_MAX (IFNAMSIZ + 6) /* '_g#_xx' */
struct txbd8
{
--
1.7.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] gianfar: Fix possible overrun and simplify interrupt name field creation
2012-03-25 17:10 ` [PATCH v2] " Paul Gortmaker
@ 2012-03-25 17:29 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2012-03-25 17:29 UTC (permalink / raw)
To: paul.gortmaker; +Cc: netdev, linux-kernel, afleming, joe
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Sun, 25 Mar 2012 13:10:07 -0400
> From: Joe Perches <joe@perches.com>
>
> Space allocated for int_name_<foo> is insufficient for
> maximal device name, expand it.
>
> Code to create int_name_<foo> is obscure, simplify it
> by using sprintf.
>
> Found by looking for unnecessary \ line continuations.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> Tested-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-03-25 17:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-17 19:05 [PATCH] gianfar: Fix possible overrun and simplify interrupt name field creation Joe Perches
2012-03-22 2:34 ` David Miller
2012-03-23 3:34 ` David Miller
2012-03-23 23:37 ` Paul Gortmaker
2012-03-25 17:10 ` [PATCH v2] " Paul Gortmaker
2012-03-25 17:29 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.