Change sizeof(x) / sizeof(*x) to ARRAY_SIZE(x).
Signed-off-by: Kulikov Vasiliy <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> > ---
> > 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.
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 <[email protected]>
> > > ---
> > > 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
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<[email protected]>
>>>> ---
>>>> 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.