* [PATCH 15/16] trivial: use ARRAY_SIZE @ 2010-06-28 11:55 Kulikov Vasiliy 2010-06-28 12:52 ` Dan Carpenter 0 siblings, 1 reply; 5+ messages in thread From: Kulikov Vasiliy @ 2010-06-28 11:55 UTC (permalink / raw) To: trivial Cc: Kernel Janitors, Greg Kroah-Hartman, Kulikov Vasiliy, Henk de Groot, Joe Perches, devel, linux-kernel Change sizeof(x) / sizeof(*x) to ARRAY_SIZE(x). Signed-off-by: Kulikov Vasiliy <segooon@gmail.com> --- drivers/staging/wlags49_h2/hcf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/staging/wlags49_h2/hcf.c b/drivers/staging/wlags49_h2/hcf.c index 390628c..c4fe0ec 100644 --- a/drivers/staging/wlags49_h2/hcf.c +++ b/drivers/staging/wlags49_h2/hcf.c @@ -502,7 +502,7 @@ HCF_STATIC hcf_16* BASED xxxx[ ] = { #endif // MSF_COMPONENT_ID NULL //endsentinel }; -#define xxxx_PRI_IDENTITY_OFFSET (sizeof(xxxx)/sizeof(xxxx[0]) - 3) +#define xxxx_PRI_IDENTITY_OFFSET (ARRAY_SIZE(xxxx) - 3) #endif // MSF_COMPONENT_ID / HCF_EXT_MB -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 15/16] trivial: use ARRAY_SIZE 2010-06-28 11:55 [PATCH 15/16] trivial: use ARRAY_SIZE Kulikov Vasiliy @ 2010-06-28 12:52 ` Dan Carpenter 2010-06-28 13:15 ` Kulikov Vasiliy 0 siblings, 1 reply; 5+ messages in thread From: Dan Carpenter @ 2010-06-28 12:52 UTC (permalink / raw) To: Kulikov Vasiliy Cc: trivial, Kernel Janitors, Greg Kroah-Hartman, Henk de Groot, Joe Perches, devel, linux-kernel On Mon, Jun 28, 2010 at 03:55:41PM +0400, Kulikov Vasiliy wrote: > Change sizeof(x) / sizeof(*x) to ARRAY_SIZE(x). > > Signed-off-by: Kulikov Vasiliy <segooon@gmail.com> > --- > drivers/staging/wlags49_h2/hcf.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/staging/wlags49_h2/hcf.c b/drivers/staging/wlags49_h2/hcf.c > index 390628c..c4fe0ec 100644 > --- a/drivers/staging/wlags49_h2/hcf.c > +++ b/drivers/staging/wlags49_h2/hcf.c > @@ -502,7 +502,7 @@ HCF_STATIC hcf_16* BASED xxxx[ ] = { > #endif // MSF_COMPONENT_ID > NULL //endsentinel > }; > -#define xxxx_PRI_IDENTITY_OFFSET (sizeof(xxxx)/sizeof(xxxx[0]) - 3) > +#define xxxx_PRI_IDENTITY_OFFSET (ARRAY_SIZE(xxxx) - 3) > I would say the more critical problem with this macro is that it doesn't work unless you name all your arrays "xxxx[]" so it encourages sub par variable names. You could do: #define PRI_IDENTITY_OFFSET(x) (ARRAY_SIZE(x) - 3) regards, dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 15/16] trivial: use ARRAY_SIZE 2010-06-28 12:52 ` Dan Carpenter @ 2010-06-28 13:15 ` Kulikov Vasiliy 2010-06-28 13:24 ` Dan Carpenter 0 siblings, 1 reply; 5+ messages in thread From: Kulikov Vasiliy @ 2010-06-28 13:15 UTC (permalink / raw) To: Dan Carpenter, trivial, Kernel Janitors, Greg Kroah-Hartman, Henk de Groot, Joe Perches, devel, linux-kernel On Mon, Jun 28, 2010 at 14:52 +0200, Dan Carpenter wrote: > On Mon, Jun 28, 2010 at 03:55:41PM +0400, Kulikov Vasiliy wrote: > > Change sizeof(x) / sizeof(*x) to ARRAY_SIZE(x). > > > > Signed-off-by: Kulikov Vasiliy <segooon@gmail.com> > > --- > > drivers/staging/wlags49_h2/hcf.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/staging/wlags49_h2/hcf.c b/drivers/staging/wlags49_h2/hcf.c > > index 390628c..c4fe0ec 100644 > > --- a/drivers/staging/wlags49_h2/hcf.c > > +++ b/drivers/staging/wlags49_h2/hcf.c > > @@ -502,7 +502,7 @@ HCF_STATIC hcf_16* BASED xxxx[ ] = { > > #endif // MSF_COMPONENT_ID > > NULL //endsentinel > > }; > > -#define xxxx_PRI_IDENTITY_OFFSET (sizeof(xxxx)/sizeof(xxxx[0]) - 3) > > +#define xxxx_PRI_IDENTITY_OFFSET (ARRAY_SIZE(xxxx) - 3) > > > > I would say the more critical problem with this macro is that it doesn't > work unless you name all your arrays "xxxx[]" so it encourages sub par > variable names. > > You could do: > #define PRI_IDENTITY_OFFSET(x) (ARRAY_SIZE(x) - 3) Look at the patch: > > @@ -502,7 +502,7 @@ HCF_STATIC hcf_16* BASED xxxx[ ] = { There is an array called 'xxxx' and macro xxxx_PRI_IDENTITY_OFFSET is defined after array definition. This magic macroconstant is used in the code to get elements of xxxx. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 15/16] trivial: use ARRAY_SIZE 2010-06-28 13:15 ` Kulikov Vasiliy @ 2010-06-28 13:24 ` Dan Carpenter 2010-06-28 20:43 ` Henk de Groot 0 siblings, 1 reply; 5+ messages in thread From: Dan Carpenter @ 2010-06-28 13:24 UTC (permalink / raw) To: Kulikov Vasiliy Cc: trivial, Kernel Janitors, Greg Kroah-Hartman, Henk de Groot, Joe Perches, devel, linux-kernel On Mon, Jun 28, 2010 at 05:15:11PM +0400, Kulikov Vasiliy wrote: > On Mon, Jun 28, 2010 at 14:52 +0200, Dan Carpenter wrote: > > On Mon, Jun 28, 2010 at 03:55:41PM +0400, Kulikov Vasiliy wrote: > > > Change sizeof(x) / sizeof(*x) to ARRAY_SIZE(x). > > > > > > Signed-off-by: Kulikov Vasiliy <segooon@gmail.com> > > > --- > > > drivers/staging/wlags49_h2/hcf.c | 2 +- > > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > > > diff --git a/drivers/staging/wlags49_h2/hcf.c b/drivers/staging/wlags49_h2/hcf.c > > > index 390628c..c4fe0ec 100644 > > > --- a/drivers/staging/wlags49_h2/hcf.c > > > +++ b/drivers/staging/wlags49_h2/hcf.c > > > @@ -502,7 +502,7 @@ HCF_STATIC hcf_16* BASED xxxx[ ] = { > > > #endif // MSF_COMPONENT_ID > > > NULL //endsentinel > > > }; > > > -#define xxxx_PRI_IDENTITY_OFFSET (sizeof(xxxx)/sizeof(xxxx[0]) - 3) > > > +#define xxxx_PRI_IDENTITY_OFFSET (ARRAY_SIZE(xxxx) - 3) > > > > > > > I would say the more critical problem with this macro is that it doesn't > > work unless you name all your arrays "xxxx[]" so it encourages sub par > > variable names. > > > > You could do: > > #define PRI_IDENTITY_OFFSET(x) (ARRAY_SIZE(x) - 3) > > Look at the patch: > > > > @@ -502,7 +502,7 @@ HCF_STATIC hcf_16* BASED xxxx[ ] = { > There is an array called 'xxxx' and macro xxxx_PRI_IDENTITY_OFFSET is defined > after array definition. This magic macroconstant is used in the code to get > elements of xxxx. Right right. But xxxx is a stupid name for a variable. I wanted to poke my eyes out with a fork. Not your fault obviously. Your patch doesn't make it worse so I'm fine with it as far as it goes... regards, dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 15/16] trivial: use ARRAY_SIZE 2010-06-28 13:24 ` Dan Carpenter @ 2010-06-28 20:43 ` Henk de Groot 0 siblings, 0 replies; 5+ messages in thread From: Henk de Groot @ 2010-06-28 20:43 UTC (permalink / raw) To: Dan Carpenter, Kulikov Vasiliy, trivial, Kernel Janitors, Greg Kroah-Hartman, Henk de Groot, Joe Perches, devel, linux-kernel Op 28-6-2010 15:24, Dan Carpenter schreef: > On Mon, Jun 28, 2010 at 05:15:11PM +0400, Kulikov Vasiliy wrote: > >> On Mon, Jun 28, 2010 at 14:52 +0200, Dan Carpenter wrote: >> >>> On Mon, Jun 28, 2010 at 03:55:41PM +0400, Kulikov Vasiliy wrote: >>> >>>> Change sizeof(x) / sizeof(*x) to ARRAY_SIZE(x). >>>> >>>> Signed-off-by: Kulikov Vasiliy<segooon@gmail.com> >>>> --- >>>> drivers/staging/wlags49_h2/hcf.c | 2 +- >>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/drivers/staging/wlags49_h2/hcf.c b/drivers/staging/wlags49_h2/hcf.c >>>> index 390628c..c4fe0ec 100644 >>>> --- a/drivers/staging/wlags49_h2/hcf.c >>>> +++ b/drivers/staging/wlags49_h2/hcf.c >>>> @@ -502,7 +502,7 @@ HCF_STATIC hcf_16* BASED xxxx[ ] = { >>>> #endif // MSF_COMPONENT_ID >>>> NULL //endsentinel >>>> }; >>>> -#define xxxx_PRI_IDENTITY_OFFSET (sizeof(xxxx)/sizeof(xxxx[0]) - 3) >>>> +#define xxxx_PRI_IDENTITY_OFFSET (ARRAY_SIZE(xxxx) - 3) >>>> >>>> >>> I would say the more critical problem with this macro is that it doesn't >>> work unless you name all your arrays "xxxx[]" so it encourages sub par >>> variable names. >>> >>> You could do: >>> #define PRI_IDENTITY_OFFSET(x) (ARRAY_SIZE(x) - 3) >>> >> Look at the patch: >> >> >>>> @@ -502,7 +502,7 @@ HCF_STATIC hcf_16* BASED xxxx[ ] = { >>>> >> There is an array called 'xxxx' and macro xxxx_PRI_IDENTITY_OFFSET is defined >> after array definition. This magic macroconstant is used in the code to get >> elements of xxxx. >> > Right right. But xxxx is a stupid name for a variable. I wanted to > poke my eyes out with a fork. > > Not your fault obviously. Your patch doesn't make it worse so I'm fine > with it as far as it goes... > > regards, > dan carpenter > The whole HCF library code is full of this funny stuff. My guess is the code is generated and macro's are used to access arrays in a uniform way. At least that's the only thing I can think of why it is as it is. Only the original developers at Agere can tell the real story. The same with structure definitions. You could send this to the IOCCC easily (it is only too big to qualify)... I think the best way to fix this is to take all this funny stuff out and make it simplified readable code. Currently its very hard to see through all the macro layers what actually happens which make is unmaintainable. Changing details is okay, but will never make any of this more readable. When porting the driver from the original Agere 2.4 kernel code to the 2.6 driver today I only need to change one or two casts, the rest of the HCF code is untouched. The only Linux code is in the wl_* files which coding style looks quite different from the rest and is actually readable. Kind regards, Henk. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-06-28 20:57 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-06-28 11:55 [PATCH 15/16] trivial: use ARRAY_SIZE Kulikov Vasiliy 2010-06-28 12:52 ` Dan Carpenter 2010-06-28 13:15 ` Kulikov Vasiliy 2010-06-28 13:24 ` Dan Carpenter 2010-06-28 20:43 ` Henk de Groot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox