powerpc/xmon: replace sizeof calculations with ARRAY_SIZE macro

[PATCH] powerpc/xmon: replace sizeof calculations with ARRAY_SIZE macro

Posted by Ruben Wauters 2 months, 2 weeks ago

The calculations for operand/opcode/macro numbers are done in an
identical manner to the already existing ARRAY_SIZE macro in
linux/array_size.h

This patch replaces the sizeof calculations with the macro to make the
code cleaner and more immediately obvious what it is doing.

Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
 arch/powerpc/xmon/ppc-opc.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
index 0774d711453e..de9b4236728c 100644
--- a/arch/powerpc/xmon/ppc-opc.c
+++ b/arch/powerpc/xmon/ppc-opc.c
@@ -954,8 +954,7 @@ const struct powerpc_operand powerpc_operands[] =
   { 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
 };
 
-const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
-					   / sizeof (powerpc_operands[0]));
+const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
 
 /* The functions used to insert and extract complicated operands.  */
 
@@ -6968,9 +6967,8 @@ const struct powerpc_opcode powerpc_opcodes[] = {
 {"fcfidu.",	XRC(63,974,1),	XRA_MASK, POWER7|PPCA2,	PPCVLE,		{FRT, FRB}},
 };
 
-const int powerpc_num_opcodes =
-  sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
-
+const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
+
 /* The VLE opcode table.
 
    The format of this opcode table is the same as the main opcode table.  */
@@ -7207,9 +7205,8 @@ const struct powerpc_opcode vle_opcodes[] = {
 {"se_bl",	BD8(58,0,1),	BD8_MASK,	PPCVLE,	0,		{B8}},
 };
 
-const int vle_num_opcodes =
-  sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
-
+const int vle_num_opcodes = ARRAY_SIZE(vle_opcodes);
+
 /* The macro table.  This is only used by the assembler.  */
 
 /* The expressions of the form (-x ! 31) & (x | 31) have the value 0
@@ -7276,5 +7273,4 @@ const struct powerpc_macro powerpc_macros[] = {
 {"e_clrlslwi",4, PPCVLE, "e_rlwinm %0,%1,%3,(%2)-(%3),31-(%3)"},
 };
 
-const int powerpc_num_macros =
-  sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
+const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
-- 
2.49.1

Re: [PATCH] powerpc/xmon: replace sizeof calculations with ARRAY_SIZE macro

Posted by Madhavan Srinivasan 1 month ago

On Sat, 19 Jul 2025 23:43:10 +0100, Ruben Wauters wrote:
> The calculations for operand/opcode/macro numbers are done in an
> identical manner to the already existing ARRAY_SIZE macro in
> linux/array_size.h
> 
> This patch replaces the sizeof calculations with the macro to make the
> code cleaner and more immediately obvious what it is doing.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/xmon: replace sizeof calculations with ARRAY_SIZE macro
      https://git.kernel.org/powerpc/c/3d9c9e1da9886d91c24d97ca20c854c29212b736

Thanks

Re: [PATCH] powerpc/xmon: replace sizeof calculations with ARRAY_SIZE macro

Posted by Christophe Leroy 2 months, 1 week ago


Le 20/07/2025 à 00:43, Ruben Wauters a écrit :
> [Vous ne recevez pas souvent de courriers de rubenru09@aol.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> The calculations for operand/opcode/macro numbers are done in an
> identical manner to the already existing ARRAY_SIZE macro in
> linux/array_size.h
> 
> This patch replaces the sizeof calculations with the macro to make the
> code cleaner and more immediately obvious what it is doing.
> 
> Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> ---
>   arch/powerpc/xmon/ppc-opc.c | 16 ++++++----------

This file is mostly taken from binutils, refer commit 08d96e0b127e 
("powerpc/xmon: Apply binutils changes to upgrade disassembly").

I think your change should be done in binutils then backported to the 
kernel. The more we diverge from binutils the worse it is.

Christophe


>   1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
> index 0774d711453e..de9b4236728c 100644
> --- a/arch/powerpc/xmon/ppc-opc.c
> +++ b/arch/powerpc/xmon/ppc-opc.c
> @@ -954,8 +954,7 @@ const struct powerpc_operand powerpc_operands[] =
>     { 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
>   };
> 
> -const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
> -                                          / sizeof (powerpc_operands[0]));
> +const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
> 
>   /* The functions used to insert and extract complicated operands.  */
> 
> @@ -6968,9 +6967,8 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>   {"fcfidu.",    XRC(63,974,1),  XRA_MASK, POWER7|PPCA2, PPCVLE,         {FRT, FRB}},
>   };
> 
> -const int powerpc_num_opcodes =
> -  sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
> -
> +const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
> +
>   /* The VLE opcode table.
> 
>      The format of this opcode table is the same as the main opcode table.  */
> @@ -7207,9 +7205,8 @@ const struct powerpc_opcode vle_opcodes[] = {
>   {"se_bl",      BD8(58,0,1),    BD8_MASK,       PPCVLE, 0,              {B8}},
>   };
> 
> -const int vle_num_opcodes =
> -  sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
> -
> +const int vle_num_opcodes = ARRAY_SIZE(vle_opcodes);
> +
>   /* The macro table.  This is only used by the assembler.  */
> 
>   /* The expressions of the form (-x ! 31) & (x | 31) have the value 0
> @@ -7276,5 +7273,4 @@ const struct powerpc_macro powerpc_macros[] = {
>   {"e_clrlslwi",4, PPCVLE, "e_rlwinm %0,%1,%3,(%2)-(%3),31-(%3)"},
>   };
> 
> -const int powerpc_num_macros =
> -  sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
> +const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
> --
> 2.49.1
>

Re: [PATCH] powerpc/xmon: replace sizeof calculations with ARRAY_SIZE macro

Posted by Ruben Wauters 2 months ago

On Wed, 2025-07-30 at 08:14 +0200, Christophe Leroy wrote:
> 
> 
> Le 20/07/2025 à 00:43, Ruben Wauters a écrit :
> > [Vous ne recevez pas souvent de courriers de rubenru09@aol.com.
> > Découvrez pourquoi ceci est important à
> > https://aka.ms/LearnAboutSenderIdentification ]
> > 
> > The calculations for operand/opcode/macro numbers are done in an
> > identical manner to the already existing ARRAY_SIZE macro in
> > linux/array_size.h
> > 
> > This patch replaces the sizeof calculations with the macro to make
> > the
> > code cleaner and more immediately obvious what it is doing.
> > 
> > Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> > ---
> >   arch/powerpc/xmon/ppc-opc.c | 16 ++++++----------
> 
> This file is mostly taken from binutils, refer commit 08d96e0b127e 
> ("powerpc/xmon: Apply binutils changes to upgrade disassembly").
> 
> I think your change should be done in binutils then backported to the
> kernel. The more we diverge from binutils the worse it is

While this is somewhat reasonable, the header (linux/array_size.h) is
linux specific, and is included through kernel.h

I can probably see if binutils has a similar header, or if they would
accept an equivalent header to make the file equivalent, but I'm unsure
if they would

Ruben

> Christophe
> 
> 
> >   1 file changed, 6 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-
> > opc.c
> > index 0774d711453e..de9b4236728c 100644
> > --- a/arch/powerpc/xmon/ppc-opc.c
> > +++ b/arch/powerpc/xmon/ppc-opc.c
> > @@ -954,8 +954,7 @@ const struct powerpc_operand powerpc_operands[]
> > =
> >     { 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
> >   };
> > 
> > -const unsigned int num_powerpc_operands = (sizeof
> > (powerpc_operands)
> > -                                          / sizeof
> > (powerpc_operands[0]));
> > +const unsigned int num_powerpc_operands =
> > ARRAY_SIZE(powerpc_operands);
> > 
> >   /* The functions used to insert and extract complicated
> > operands.  */
> > 
> > @@ -6968,9 +6967,8 @@ const struct powerpc_opcode powerpc_opcodes[]
> > = {
> >   {"fcfidu.",    XRC(63,974,1),  XRA_MASK, POWER7|PPCA2,
> > PPCVLE,         {FRT, FRB}},
> >   };
> > 
> > -const int powerpc_num_opcodes =
> > -  sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
> > -
> > +const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
> > +
> >   /* The VLE opcode table.
> > 
> >      The format of this opcode table is the same as the main opcode
> > table.  */
> > @@ -7207,9 +7205,8 @@ const struct powerpc_opcode vle_opcodes[] = {
> >   {"se_bl",      BD8(58,0,1),    BD8_MASK,       PPCVLE,
> > 0,              {B8}},
> >   };
> > 
> > -const int vle_num_opcodes =
> > -  sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
> > -
> > +const int vle_num_opcodes = ARRAY_SIZE(vle_opcodes);
> > +
> >   /* The macro table.  This is only used by the assembler.  */
> > 
> >   /* The expressions of the form (-x ! 31) & (x | 31) have the
> > value 0
> > @@ -7276,5 +7273,4 @@ const struct powerpc_macro powerpc_macros[] =
> > {
> >   {"e_clrlslwi",4, PPCVLE, "e_rlwinm %0,%1,%3,(%2)-(%3),31-(%3)"},
> >   };
> > 
> > -const int powerpc_num_macros =
> > -  sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
> > +const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
> > --
> > 2.49.1
> >

Re: [PATCH] powerpc/xmon: replace sizeof calculations with ARRAY_SIZE macro

Posted by Dan Carpenter 2 months ago

Can't hurt to try at least.

If they don't merge it, then for code where we're not upstream, it's
pretty common to just endure style issues.  I'm thinking of ACPI here.

regards,
dan carpenter

Re: [PATCH] powerpc/xmon: replace sizeof calculations with ARRAY_SIZE macro

Posted by Ruben Wauters 1 month, 4 weeks ago

On Thu, 2025-07-31 at 17:40 +0100, Ruben Wauters wrote:
> On Wed, 2025-07-30 at 08:14 +0200, Christophe Leroy wrote:
> > 
> > 
> > Le 20/07/2025 à 00:43, Ruben Wauters a écrit :
> > > [Vous ne recevez pas souvent de courriers de rubenru09@aol.com.
> > > Découvrez pourquoi ceci est important à
> > > https://aka.ms/LearnAboutSenderIdentification ]
> > > 
> > > The calculations for operand/opcode/macro numbers are done in an
> > > identical manner to the already existing ARRAY_SIZE macro in
> > > linux/array_size.h
> > > 
> > > This patch replaces the sizeof calculations with the macro to
> > > make
> > > the
> > > code cleaner and more immediately obvious what it is doing.
> > > 
> > > Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> > > ---
> > >   arch/powerpc/xmon/ppc-opc.c | 16 ++++++----------
> > 
> > This file is mostly taken from binutils, refer commit 08d96e0b127e 
> > ("powerpc/xmon: Apply binutils changes to upgrade disassembly").
> > 
> > I think your change should be done in binutils then backported to
> > the
> > kernel. The more we diverge from binutils the worse it is
> 
> While this is somewhat reasonable, the header (linux/array_size.h) is
> linux specific, and is included through kernel.h
> 
> I can probably see if binutils has a similar header, or if they would
> accept an equivalent header to make the file equivalent, but I'm
> unsure
> if they would
> 
Hello, I have checked the binutils source, and have noticed that
binutils already uses the ARRAY_SIZE macro, and this patch actually
brings the kernel more in line with the equivalent file in binutils
(opcodes/ppc-opc.c)

As such, I would like to request that the patch be applied as-is.

Thank you

Ruben

> Ruben
> 
> > Christophe
> > 
> > 
> > >   1 file changed, 6 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-
> > > opc.c
> > > index 0774d711453e..de9b4236728c 100644
> > > --- a/arch/powerpc/xmon/ppc-opc.c
> > > +++ b/arch/powerpc/xmon/ppc-opc.c
> > > @@ -954,8 +954,7 @@ const struct powerpc_operand
> > > powerpc_operands[]
> > > =
> > >     { 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
> > >   };
> > > 
> > > -const unsigned int num_powerpc_operands = (sizeof
> > > (powerpc_operands)
> > > -                                          / sizeof
> > > (powerpc_operands[0]));
> > > +const unsigned int num_powerpc_operands =
> > > ARRAY_SIZE(powerpc_operands);
> > > 
> > >   /* The functions used to insert and extract complicated
> > > operands.  */
> > > 
> > > @@ -6968,9 +6967,8 @@ const struct powerpc_opcode
> > > powerpc_opcodes[]
> > > = {
> > >   {"fcfidu.",    XRC(63,974,1),  XRA_MASK, POWER7|PPCA2,
> > > PPCVLE,         {FRT, FRB}},
> > >   };
> > > 
> > > -const int powerpc_num_opcodes =
> > > -  sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
> > > -
> > > +const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
> > > +
> > >   /* The VLE opcode table.
> > > 
> > >      The format of this opcode table is the same as the main
> > > opcode
> > > table.  */
> > > @@ -7207,9 +7205,8 @@ const struct powerpc_opcode vle_opcodes[] =
> > > {
> > >   {"se_bl",      BD8(58,0,1),    BD8_MASK,       PPCVLE,
> > > 0,              {B8}},
> > >   };
> > > 
> > > -const int vle_num_opcodes =
> > > -  sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
> > > -
> > > +const int vle_num_opcodes = ARRAY_SIZE(vle_opcodes);
> > > +
> > >   /* The macro table.  This is only used by the assembler.  */
> > > 
> > >   /* The expressions of the form (-x ! 31) & (x | 31) have the
> > > value 0
> > > @@ -7276,5 +7273,4 @@ const struct powerpc_macro powerpc_macros[]
> > > =
> > > {
> > >   {"e_clrlslwi",4, PPCVLE, "e_rlwinm %0,%1,%3,(%2)-(%3),31-
> > > (%3)"},
> > >   };
> > > 
> > > -const int powerpc_num_macros =
> > > -  sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
> > > +const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
> > > --
> > > 2.49.1
> > >

Re: [PATCH] powerpc/xmon: replace sizeof calculations with ARRAY_SIZE macro

Posted by Christophe Leroy 1 month, 4 weeks ago


Le 08/08/2025 à 15:54, Ruben Wauters a écrit :
> [Vous ne recevez pas souvent de courriers de rubenru09@aol.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Thu, 2025-07-31 at 17:40 +0100, Ruben Wauters wrote:
>> On Wed, 2025-07-30 at 08:14 +0200, Christophe Leroy wrote:
>>>
>>>
>>> Le 20/07/2025 à 00:43, Ruben Wauters a écrit :
>>>> [Vous ne recevez pas souvent de courriers de rubenru09@aol.com.
>>>> Découvrez pourquoi ceci est important à
>>>> https://aka.ms/LearnAboutSenderIdentification ]
>>>>
>>>> The calculations for operand/opcode/macro numbers are done in an
>>>> identical manner to the already existing ARRAY_SIZE macro in
>>>> linux/array_size.h
>>>>
>>>> This patch replaces the sizeof calculations with the macro to
>>>> make
>>>> the
>>>> code cleaner and more immediately obvious what it is doing.
>>>>
>>>> Signed-off-by: Ruben Wauters <rubenru09@aol.com>
>>>> ---
>>>>    arch/powerpc/xmon/ppc-opc.c | 16 ++++++----------
>>>
>>> This file is mostly taken from binutils, refer commit 08d96e0b127e
>>> ("powerpc/xmon: Apply binutils changes to upgrade disassembly").
>>>
>>> I think your change should be done in binutils then backported to
>>> the
>>> kernel. The more we diverge from binutils the worse it is
>>
>> While this is somewhat reasonable, the header (linux/array_size.h) is
>> linux specific, and is included through kernel.h
>>
>> I can probably see if binutils has a similar header, or if they would
>> accept an equivalent header to make the file equivalent, but I'm
>> unsure
>> if they would
>>
> Hello, I have checked the binutils source, and have noticed that
> binutils already uses the ARRAY_SIZE macro, and this patch actually
> brings the kernel more in line with the equivalent file in binutils
> (opcodes/ppc-opc.c)

Ok.


Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>



> 
> As such, I would like to request that the patch be applied as-is.
> 
> Thank you
> 
> Ruben
> 
>> Ruben
>>
>>> Christophe
>>>
>>>
>>>>    1 file changed, 6 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-
>>>> opc.c
>>>> index 0774d711453e..de9b4236728c 100644
>>>> --- a/arch/powerpc/xmon/ppc-opc.c
>>>> +++ b/arch/powerpc/xmon/ppc-opc.c
>>>> @@ -954,8 +954,7 @@ const struct powerpc_operand
>>>> powerpc_operands[]
>>>> =
>>>>      { 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
>>>>    };
>>>>
>>>> -const unsigned int num_powerpc_operands = (sizeof
>>>> (powerpc_operands)
>>>> -                                          / sizeof
>>>> (powerpc_operands[0]));
>>>> +const unsigned int num_powerpc_operands =
>>>> ARRAY_SIZE(powerpc_operands);
>>>>
>>>>    /* The functions used to insert and extract complicated
>>>> operands.  */
>>>>
>>>> @@ -6968,9 +6967,8 @@ const struct powerpc_opcode
>>>> powerpc_opcodes[]
>>>> = {
>>>>    {"fcfidu.",    XRC(63,974,1),  XRA_MASK, POWER7|PPCA2,
>>>> PPCVLE,         {FRT, FRB}},
>>>>    };
>>>>
>>>> -const int powerpc_num_opcodes =
>>>> -  sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
>>>> -
>>>> +const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
>>>> +
>>>>    /* The VLE opcode table.
>>>>
>>>>       The format of this opcode table is the same as the main
>>>> opcode
>>>> table.  */
>>>> @@ -7207,9 +7205,8 @@ const struct powerpc_opcode vle_opcodes[] =
>>>> {
>>>>    {"se_bl",      BD8(58,0,1),    BD8_MASK,       PPCVLE,
>>>> 0,              {B8}},
>>>>    };
>>>>
>>>> -const int vle_num_opcodes =
>>>> -  sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
>>>> -
>>>> +const int vle_num_opcodes = ARRAY_SIZE(vle_opcodes);
>>>> +
>>>>    /* The macro table.  This is only used by the assembler.  */
>>>>
>>>>    /* The expressions of the form (-x ! 31) & (x | 31) have the
>>>> value 0
>>>> @@ -7276,5 +7273,4 @@ const struct powerpc_macro powerpc_macros[]
>>>> =
>>>> {
>>>>    {"e_clrlslwi",4, PPCVLE, "e_rlwinm %0,%1,%3,(%2)-(%3),31-
>>>> (%3)"},
>>>>    };
>>>>
>>>> -const int powerpc_num_macros =
>>>> -  sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
>>>> +const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
>>>> --
>>>> 2.49.1
>>>>