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

[Christophe Leroy <christophe.leroy@csgroup.eu>]

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
>>>>