* Re: 2.6.21-rc2-mm2: drivers/net/wireless/libertas/debugfs.c addr bogosity
[not found] ` <20070309021146.GL4575@bakeyournoodle.com>
@ 2007-03-09 14:19 ` John W. Linville
2007-03-09 17:14 ` Randy Dunlap
0 siblings, 1 reply; 3+ messages in thread
From: John W. Linville @ 2007-03-09 14:19 UTC (permalink / raw)
To: Andrew Morton, Adrian Bunk, jgarzik, linux-kernel, netdev; +Cc: linux-wireless
Probably should copy me and linux-wireless@vger.kernel.org. Other than
that, this looks fine to me. I'll queue it up shortly.
John
On Fri, Mar 09, 2007 at 01:11:46PM +1100, Tony Breeds wrote:
> On Wed, Mar 07, 2007 at 03:00:57PM -0800, Andrew Morton wrote:
> > On Wed, 7 Mar 2007 23:41:16 +0100
> > Adrian Bunk <bunk@stusta.de> wrote:
> >
> > > On Tue, Mar 06, 2007 at 12:44:08AM -0800, Andrew Morton wrote:
> > > >...
> > > > Changes since 2.6.20-rc2-mm1:
> > > >...
> > > > git-netdev-all.patch
> > > >...
> > > > git trees
> > > >...
> > >
> > > drivers/net/wireless/libertas/debugfs.c contains:
> > > #define item_addr(n) ((u32) &((wlan_adapter *)0)->n)
> > >
> > > This is wrong on 64bit architectures.
> >
> > Is OK - it is simply yet another reimplementation of offsetof(). Hint.
>
> Perhaps something like?
>
> Against 2.6.21-rc3-mm2
>
> From: Tony Breeds <tony@bakeyournoodle.com>
>
> Cleanup drivers/net/wireless/libertas/debugfs.c to use standard kernel macros and functions.
>
> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
>
> ---
> only compile tested on x86
>
> debugfs.c | 41 ++++-------------------------------------
> 1 file changed, 4 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/debugfs.c b/drivers/net/wireless/libertas/debugfs.c
> index 3ad1e03..51dfd20 100644
> --- a/drivers/net/wireless/libertas/debugfs.c
> +++ b/drivers/net/wireless/libertas/debugfs.c
> @@ -1772,8 +1772,8 @@ void libertas_debugfs_remove_one(wlan_private *priv)
>
> /* debug entry */
>
> -#define item_size(n) (sizeof ((wlan_adapter *)0)->n)
> -#define item_addr(n) ((u32) &((wlan_adapter *)0)->n)
> +#define item_size(n) (FIELD_SIZEOF(wlan_adapter, n))
> +#define item_addr(n) (offsetof(wlan_adapter, n))
>
> struct debug_data {
> char name[32];
> @@ -1789,40 +1789,7 @@ static struct debug_data items[] = {
> {"psstate", item_size(psstate), item_addr(psstate)},
> };
>
> -static int num_of_items = sizeof(items) / sizeof(items[0]);
> -
> -/**
> - * @brief convert string to number
> - *
> - * @param s pointer to numbered string
> - * @return converted number from string s
> - */
> -static int string_to_number(char *s)
> -{
> - int r = 0;
> - int base = 0;
> -
> - if ((strncmp(s, "0x", 2) == 0) || (strncmp(s, "0X", 2) == 0))
> - base = 16;
> - else
> - base = 10;
> -
> - if (base == 16)
> - s += 2;
> -
> - for (s = s; *s != 0; s++) {
> - if ((*s >= 48) && (*s <= 57))
> - r = (r * base) + (*s - 48);
> - else if ((*s >= 65) && (*s <= 70))
> - r = (r * base) + (*s - 55);
> - else if ((*s >= 97) && (*s <= 102))
> - r = (r * base) + (*s - 87);
> - else
> - break;
> - }
> -
> - return r;
> -}
> +static int num_of_items = ARRAY_SIZE(items);
>
> /**
> * @brief proc read function
> @@ -1912,7 +1879,7 @@ static int wlan_debugfs_write(struct file *f, const char __user *buf,
> if (!p2)
> break;
> p2++;
> - r = string_to_number(p2);
> + r = simple_strtoul(p2, NULL, 0);
> if (d[i].size == 1)
> *((u8 *) d[i].addr) = (u8) r;
> else if (d[i].size == 2)
> Yours Tony
>
> linux.conf.au http://linux.conf.au/ || http://lca2008.linux.org.au/
> Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
John W. Linville
linville@tuxdriver.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: 2.6.21-rc2-mm2: drivers/net/wireless/libertas/debugfs.c addr bogosity
2007-03-09 14:19 ` 2.6.21-rc2-mm2: drivers/net/wireless/libertas/debugfs.c addr bogosity John W. Linville
@ 2007-03-09 17:14 ` Randy Dunlap
2007-03-12 3:43 ` Tony Breeds
0 siblings, 1 reply; 3+ messages in thread
From: Randy Dunlap @ 2007-03-09 17:14 UTC (permalink / raw)
To: John W. Linville
Cc: Andrew Morton, Adrian Bunk, jgarzik, linux-kernel, netdev,
linux-wireless
On Fri, 9 Mar 2007 09:19:07 -0500 John W. Linville wrote:
> Probably should copy me and linux-wireless@vger.kernel.org. Other than
> that, this looks fine to me. I'll queue it up shortly.
>
> John
>
> On Fri, Mar 09, 2007 at 01:11:46PM +1100, Tony Breeds wrote:
> > On Wed, Mar 07, 2007 at 03:00:57PM -0800, Andrew Morton wrote:
> > > On Wed, 7 Mar 2007 23:41:16 +0100
> > > Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > > On Tue, Mar 06, 2007 at 12:44:08AM -0800, Andrew Morton wrote:
> > > > >...
> > > > > Changes since 2.6.20-rc2-mm1:
> > > > >...
> > > > > git-netdev-all.patch
> > > > >...
> > > > > git trees
> > > > >...
> > > >
> > > > drivers/net/wireless/libertas/debugfs.c contains:
> > > > #define item_addr(n) ((u32) &((wlan_adapter *)0)->n)
> > > >
> > > > This is wrong on 64bit architectures.
> > >
> > > Is OK - it is simply yet another reimplementation of offsetof(). Hint.
> >
> > Perhaps something like?
> >
> > Against 2.6.21-rc3-mm2
> >
> > From: Tony Breeds <tony@bakeyournoodle.com>
> >
> > Cleanup drivers/net/wireless/libertas/debugfs.c to use standard kernel macros and functions.
> >
> > Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
> >
> > ---
> > only compile tested on x86
> >
> > debugfs.c | 41 ++++-------------------------------------
> > 1 file changed, 4 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/net/wireless/libertas/debugfs.c b/drivers/net/wireless/libertas/debugfs.c
> > index 3ad1e03..51dfd20 100644
> > --- a/drivers/net/wireless/libertas/debugfs.c
> > +++ b/drivers/net/wireless/libertas/debugfs.c
> > @@ -1772,8 +1772,8 @@ void libertas_debugfs_remove_one(wlan_private *priv)
> >
> > /* debug entry */
> >
> > -#define item_size(n) (sizeof ((wlan_adapter *)0)->n)
> > -#define item_addr(n) ((u32) &((wlan_adapter *)0)->n)
> > +#define item_size(n) (FIELD_SIZEOF(wlan_adapter, n))
Good to use FIELD_SIZEOF(), but in general, we prefer to use it
directly, not in yet another wrapper.
> > +#define item_addr(n) (offsetof(wlan_adapter, n))
> >
> > struct debug_data {
> > char name[32];
> > @@ -1789,40 +1789,7 @@ static struct debug_data items[] = {
> > {"psstate", item_size(psstate), item_addr(psstate)},
> > };
> >
> > -static int num_of_items = sizeof(items) / sizeof(items[0]);
> > -
> > -/**
> > - * @brief convert string to number
> > - *
> > - * @param s pointer to numbered string
> > - * @return converted number from string s
> > - */
> > -static int string_to_number(char *s)
> > -{
> > - int r = 0;
> > - int base = 0;
> > -
> > - if ((strncmp(s, "0x", 2) == 0) || (strncmp(s, "0X", 2) == 0))
> > - base = 16;
> > - else
> > - base = 10;
> > -
> > - if (base == 16)
> > - s += 2;
> > -
> > - for (s = s; *s != 0; s++) {
> > - if ((*s >= 48) && (*s <= 57))
> > - r = (r * base) + (*s - 48);
> > - else if ((*s >= 65) && (*s <= 70))
> > - r = (r * base) + (*s - 55);
> > - else if ((*s >= 97) && (*s <= 102))
> > - r = (r * base) + (*s - 87);
> > - else
> > - break;
> > - }
> > -
> > - return r;
> > -}
> > +static int num_of_items = ARRAY_SIZE(items);
> >
> > /**
> > * @brief proc read function
> > @@ -1912,7 +1879,7 @@ static int wlan_debugfs_write(struct file *f, const char __user *buf,
> > if (!p2)
> > break;
> > p2++;
> > - r = string_to_number(p2);
> > + r = simple_strtoul(p2, NULL, 0);
> > if (d[i].size == 1)
> > *((u8 *) d[i].addr) = (u8) r;
> > else if (d[i].size == 2)
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: 2.6.21-rc2-mm2: drivers/net/wireless/libertas/debugfs.c addr bogosity
2007-03-09 17:14 ` Randy Dunlap
@ 2007-03-12 3:43 ` Tony Breeds
0 siblings, 0 replies; 3+ messages in thread
From: Tony Breeds @ 2007-03-12 3:43 UTC (permalink / raw)
To: Randy Dunlap
Cc: John W. Linville, Andrew Morton, Adrian Bunk, jgarzik,
linux-kernel, netdev, linux-wireless
On Fri, Mar 09, 2007 at 09:14:29AM -0800, Randy Dunlap wrote:
> Good to use FIELD_SIZEOF(),
Thanks.
> but in general, we prefer to use it
> directly, not in yet another wrapper.
I left the item_{size,addr} in place as it seemed to make the item[]
more compact.
I'm not certain using the FIELD_SIZEOF() macro directly is a win.
From: Tony Breeds <tony@bakeyournoodle.com>
Cleanup drivers/net/wireless/libertas/debugfs.c to use standard kernel macros and functions.
Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
---
only compile tested on x86
drivers/net/wireless/libertas/debugfs.c | 56 +++++++------------------------
1 files changed, 12 insertions(+), 44 deletions(-)
diff --git a/drivers/net/wireless/libertas/debugfs.c b/drivers/net/wireless/libertas/debugfs.c
index 3ad1e03..8b0e3ec 100644
--- a/drivers/net/wireless/libertas/debugfs.c
+++ b/drivers/net/wireless/libertas/debugfs.c
@@ -1771,58 +1771,26 @@ void libertas_debugfs_remove_one(wlan_private *priv)
}
/* debug entry */
-
-#define item_size(n) (sizeof ((wlan_adapter *)0)->n)
-#define item_addr(n) ((u32) &((wlan_adapter *)0)->n)
-
struct debug_data {
char name[32];
u32 size;
u32 addr;
};
-/* To debug any member of wlan_adapter, simply add one line here.
- */
+/* To debug any member of wlan_adapter, simply add a record here. */
static struct debug_data items[] = {
- {"intcounter", item_size(intcounter), item_addr(intcounter)},
- {"psmode", item_size(psmode), item_addr(psmode)},
- {"psstate", item_size(psstate), item_addr(psstate)},
+ { .name = "intcounter",
+ .size = FIELD_SIZEOF(wlan_adapter, intcounter),
+ .addr = offsetof(wlan_adapter, intcounter) },
+ { .name = "psmode",
+ .size = FIELD_SIZEOF(wlan_adapter, psmode),
+ .addr = offsetof(wlan_adapter, psmode) },
+ { .name = "psstate",
+ .size = FIELD_SIZEOF(wlan_adapter, psstate),
+ .addr = offsetof(wlan_adapter, psstate) },
};
-static int num_of_items = sizeof(items) / sizeof(items[0]);
-
-/**
- * @brief convert string to number
- *
- * @param s pointer to numbered string
- * @return converted number from string s
- */
-static int string_to_number(char *s)
-{
- int r = 0;
- int base = 0;
-
- if ((strncmp(s, "0x", 2) == 0) || (strncmp(s, "0X", 2) == 0))
- base = 16;
- else
- base = 10;
-
- if (base == 16)
- s += 2;
-
- for (s = s; *s != 0; s++) {
- if ((*s >= 48) && (*s <= 57))
- r = (r * base) + (*s - 48);
- else if ((*s >= 65) && (*s <= 70))
- r = (r * base) + (*s - 55);
- else if ((*s >= 97) && (*s <= 102))
- r = (r * base) + (*s - 87);
- else
- break;
- }
-
- return r;
-}
+static int num_of_items = ARRAY_SIZE(items);
/**
* @brief proc read function
@@ -1912,7 +1880,7 @@ static int wlan_debugfs_write(struct file *f, const char __user *buf,
if (!p2)
break;
p2++;
- r = string_to_number(p2);
+ r = simple_strtoul(p2, NULL, 0);
if (d[i].size == 1)
*((u8 *) d[i].addr) = (u8) r;
else if (d[i].size == 2)
Yours Tony
linux.conf.au http://linux.conf.au/ || http://lca2008.linux.org.au/
Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-03-12 4:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20070306004408.d3f6434d.akpm@linux-foundation.org>
[not found] ` <20070307224116.GE3441@stusta.de>
[not found] ` <20070307150057.4e6342dc.akpm@linux-foundation.org>
[not found] ` <20070309021146.GL4575@bakeyournoodle.com>
2007-03-09 14:19 ` 2.6.21-rc2-mm2: drivers/net/wireless/libertas/debugfs.c addr bogosity John W. Linville
2007-03-09 17:14 ` Randy Dunlap
2007-03-12 3:43 ` Tony Breeds
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).