2018-08-14 03:00:27

by zhong jiang

[permalink] [raw]
Subject: [PATCH 0/2] Use ARRAY_SIZE to replace its implementation

The issue is detected with the help of Coccinelle.

zhong jiang (2):
ia64: Use ARRAY_SIZE to replace its implementation
powerpc: Use ARRAY_SIZE to replace its implementation

arch/ia64/kernel/perfmon.c | 2 +-
arch/powerpc/xmon/ppc-opc.c | 12 ++++--------
2 files changed, 5 insertions(+), 9 deletions(-)

--
1.7.12.4



2018-08-14 03:00:28

by zhong jiang

[permalink] [raw]
Subject: [PATCH 2/2] powerpc: Use ARRAY_SIZE to replace its implementation

Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element.
So just replace it.

Signed-off-by: zhong jiang <[email protected]>
---
arch/powerpc/xmon/ppc-opc.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
index ac2b55b..f3f57a1 100644
--- a/arch/powerpc/xmon/ppc-opc.c
+++ b/arch/powerpc/xmon/ppc-opc.c
@@ -966,8 +966,7 @@
{ 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. */

@@ -6980,8 +6979,7 @@
{"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.

@@ -7219,8 +7217,7 @@
{"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. */

@@ -7288,5 +7285,4 @@
{"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);
--
1.7.12.4


2018-08-14 03:01:40

by zhong jiang

[permalink] [raw]
Subject: [PATCH 1/2] ia64: Use ARRAY_SIZE to replace its implementation

We prefer to ARRAY_SIZE rather than duplicating its implementation.
So just replace it.

Signed-off-by: zhong jiang <[email protected]>
---
arch/ia64/kernel/perfmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index a9d4dc6..6cbe6e0 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -4645,7 +4645,7 @@ static char *pfmfs_dname(struct dentry *dentry, char *buffer, int buflen)
/* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL),
/* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL)
};
-#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
+#define PFM_CMD_COUNT ARRAY_SIZE(pfm_cmd_tab)

static int
pfm_check_task_state(pfm_context_t *ctx, int cmd, unsigned long flags)
--
1.7.12.4


2018-08-14 04:56:29

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/2] ia64: Use ARRAY_SIZE to replace its implementation

On Tue, 2018-08-14 at 10:46 +0800, zhong jiang wrote:
> We prefer to ARRAY_SIZE rather than duplicating its implementation.
> So just replace it.
[]
> diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
[]
> @@ -4645,7 +4645,7 @@ static char *pfmfs_dname(struct dentry *dentry, char *buffer, int buflen)
> /* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL),
> /* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL)
> };
> -#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
> +#define PFM_CMD_COUNT ARRAY_SIZE(pfm_cmd_tab)

Better would be to remove the #define altogether and change
the one place where it's used to ARRAY_SIZE(...)
---
arch/ia64/kernel/perfmon.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index a9d4dc6c0427..08ece2c7b6e1 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -4645,7 +4645,6 @@ static pfm_cmd_desc_t pfm_cmd_tab[]={
/* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL),
/* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL)
};
-#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))

static int
pfm_check_task_state(pfm_context_t *ctx, int cmd, unsigned long flags)
@@ -4770,7 +4769,7 @@ sys_perfmonctl (int fd, int cmd, void __user *arg, int count)
*/
if (unlikely(pmu_conf == NULL)) return -ENOSYS;

- if (unlikely(cmd < 0 || cmd >= PFM_CMD_COUNT)) {
+ if (unlikely(cmd < 0 || cmd >= ARRAY_SIZE(pfm_cmd_tab)) {
DPRINT(("invalid cmd=%d\n", cmd));
return -EINVAL;
}


2018-08-14 05:11:59

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 2/2] powerpc: Use ARRAY_SIZE to replace its implementation

On Tue, 2018-08-14 at 10:46 +0800, zhong jiang wrote:
> Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element.
> So just replace it.

Better to remove the extern and the const altogether here as well.

$ git grep -w powerpc_num_opcodes
arch/powerpc/xmon/ppc-dis.c: opcode_end = powerpc_opcodes + powerpc_num_opcodes;
arch/powerpc/xmon/ppc-opc.c:const int powerpc_num_opcodes =
arch/powerpc/xmon/ppc.h:extern const int powerpc_num_opcodes;

And this one could be removed instead:

$ git grep -w vle_num_opcodes
arch/powerpc/xmon/ppc-opc.c:const int vle_num_opcodes =
arch/powerpc/xmon/ppc.h:extern const int vle_num_opcodes;

> Signed-off-by: zhong jiang <[email protected]>
> ---
> arch/powerpc/xmon/ppc-opc.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
> index ac2b55b..f3f57a1 100644
> --- a/arch/powerpc/xmon/ppc-opc.c
> +++ b/arch/powerpc/xmon/ppc-opc.c
> @@ -966,8 +966,7 @@
> { 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. */
>
> @@ -6980,8 +6979,7 @@
> {"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.
>
> @@ -7219,8 +7217,7 @@
> {"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. */
>
> @@ -7288,5 +7285,4 @@
> {"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);

2018-08-14 05:12:28

by zhong jiang

[permalink] [raw]
Subject: Re: [PATCH 1/2] ia64: Use ARRAY_SIZE to replace its implementation

On 2018/8/14 12:45, Joe Perches wrote:
> On Tue, 2018-08-14 at 10:46 +0800, zhong jiang wrote:
>> We prefer to ARRAY_SIZE rather than duplicating its implementation.
>> So just replace it.
> []
>> diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
> []
>> @@ -4645,7 +4645,7 @@ static char *pfmfs_dname(struct dentry *dentry, char *buffer, int buflen)
>> /* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL),
>> /* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL)
>> };
>> -#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
>> +#define PFM_CMD_COUNT ARRAY_SIZE(pfm_cmd_tab)
> Better would be to remove the #define altogether and change
> the one place where it's used to ARRAY_SIZE(...)
> ---
> arch/ia64/kernel/perfmon.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
> index a9d4dc6c0427..08ece2c7b6e1 100644
> --- a/arch/ia64/kernel/perfmon.c
> +++ b/arch/ia64/kernel/perfmon.c
> @@ -4645,7 +4645,6 @@ static pfm_cmd_desc_t pfm_cmd_tab[]={
> /* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL),
> /* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL)
> };
> -#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
>
> static int
> pfm_check_task_state(pfm_context_t *ctx, int cmd, unsigned long flags)
> @@ -4770,7 +4769,7 @@ sys_perfmonctl (int fd, int cmd, void __user *arg, int count)
> */
> if (unlikely(pmu_conf == NULL)) return -ENOSYS;
>
> - if (unlikely(cmd < 0 || cmd >= PFM_CMD_COUNT)) {
> + if (unlikely(cmd < 0 || cmd >= ARRAY_SIZE(pfm_cmd_tab)) {
> DPRINT(("invalid cmd=%d\n", cmd));
> return -EINVAL;
> }
>
>
> .
>
Thank you for suggestion. That's indeed better if just one palce use it. I will repost in v2.

Sincerely,
zhong jiang


2018-08-14 09:31:55

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH 2/2] powerpc: Use ARRAY_SIZE to replace its implementation

zhong jiang <[email protected]> writes:
> Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element.
> So just replace it.
>
> Signed-off-by: zhong jiang <[email protected]>
> ---
> arch/powerpc/xmon/ppc-opc.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)

This code is copied from binutils and we don't want to needlessly cause
it to diverge from the binutils copy.

So thanks but no thanks.

cheers

> diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
> index ac2b55b..f3f57a1 100644
> --- a/arch/powerpc/xmon/ppc-opc.c
> +++ b/arch/powerpc/xmon/ppc-opc.c
> @@ -966,8 +966,7 @@
> { 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. */
>
> @@ -6980,8 +6979,7 @@
> {"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.
>
> @@ -7219,8 +7217,7 @@
> {"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. */
>
> @@ -7288,5 +7285,4 @@
> {"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);
> --
> 1.7.12.4

2018-08-14 10:30:28

by zhong jiang

[permalink] [raw]
Subject: Re: [PATCH 2/2] powerpc: Use ARRAY_SIZE to replace its implementation

On 2018/8/14 17:28, Michael Ellerman wrote:
> zhong jiang <[email protected]> writes:
>> Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element.
>> So just replace it.
>>
>> Signed-off-by: zhong jiang <[email protected]>
>> ---
>> arch/powerpc/xmon/ppc-opc.c | 12 ++++--------
>> 1 file changed, 4 insertions(+), 8 deletions(-)
> This code is copied from binutils and we don't want to needlessly cause
> it to diverge from the binutils copy.
>
> So thanks but no thanks.
Thank you for clarification.

Sincerely
zhong jiang
> cheers
>
>> diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
>> index ac2b55b..f3f57a1 100644
>> --- a/arch/powerpc/xmon/ppc-opc.c
>> +++ b/arch/powerpc/xmon/ppc-opc.c
>> @@ -966,8 +966,7 @@
>> { 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. */
>>
>> @@ -6980,8 +6979,7 @@
>> {"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.
>>
>> @@ -7219,8 +7217,7 @@
>> {"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. */
>>
>> @@ -7288,5 +7285,4 @@
>> {"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);
>> --
>> 1.7.12.4
> .
>



2018-08-14 16:19:37

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 0/2] Use ARRAY_SIZE to replace its implementation

> "Use ARRAY_SIZE to replace its implementation"

Um, the subject line doesn't make sense.

David