[PATCH 0/2] s390: Use ARRAY_SIZE instead of reimplementing its function

[PATCH 0/2] s390: Use ARRAY_SIZE instead of reimplementing its function

[zhong jiang]

I find the issue with the help of Coccinelle.

zhong jiang (2):
  s390: vmlogrdr: Use ARRAY_SIZE instead of reimplementing its function
  s390: qeth_core_mpc: Use ARRAY_SIZE instead of reimplementing its
    function

 drivers/s390/char/vmlogrdr.c     | 2 +-
 drivers/s390/net/qeth_core_mpc.c | 7 ++-----
 2 files changed, 3 insertions(+), 6 deletions(-)

-- 
1.7.12.4

[PATCH 1/2] s390: vmlogrdr: Use ARRAY_SIZE instead of reimplementing its function

[zhong jiang]

ARRAY_SIZE has implemented its function. we prefer to use the function
rather than the open code.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/s390/char/vmlogrdr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/s390/char/vmlogrdr.c b/drivers/s390/char/vmlogrdr.c
index 069b9ef..58333cb 100644
--- a/drivers/s390/char/vmlogrdr.c
+++ b/drivers/s390/char/vmlogrdr.c
@@ -153,7 +153,7 @@ static void vmlogrdr_iucv_message_pending(struct iucv_path *,
 	}
 };
 
-#define MAXMINOR  (sizeof(sys_ser)/sizeof(struct vmlogrdr_priv_t))
+#define MAXMINOR  ARRAY_SIZE(sys_ser)
 
 static char FENCE[] = {"EOR"};
 static int vmlogrdr_major = 0;
-- 
1.7.12.4

Re: [PATCH 1/2] s390: vmlogrdr: Use ARRAY_SIZE instead of reimplementing its function

[Jean Delvare]

On Sat, 8 Sep 2018 18:26:27 +0800, zhong jiang wrote:
> ARRAY_SIZE has implemented its function. we prefer to use the function
> rather than the open code.
> 
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
>  drivers/s390/char/vmlogrdr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/s390/char/vmlogrdr.c b/drivers/s390/char/vmlogrdr.c
> index 069b9ef..58333cb 100644
> --- a/drivers/s390/char/vmlogrdr.c
> +++ b/drivers/s390/char/vmlogrdr.c
> @@ -153,7 +153,7 @@ static void vmlogrdr_iucv_message_pending(struct iucv_path *,
>  	}
>  };
>  
> -#define MAXMINOR  (sizeof(sys_ser)/sizeof(struct vmlogrdr_priv_t))
> +#define MAXMINOR  ARRAY_SIZE(sys_ser)
>  
>  static char FENCE[] = {"EOR"};
>  static int vmlogrdr_major = 0;

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support

[PATCH 2/2] s390: qeth_core_mpc: Use ARRAY_SIZE instead of reimplementing its function

[zhong jiang]

ARRAY_SIZE has implemented its function. we prefer to use the function
rather than the open code.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/s390/net/qeth_core_mpc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/s390/net/qeth_core_mpc.c b/drivers/s390/net/qeth_core_mpc.c
index 5bcb8da..e8263de 100644
--- a/drivers/s390/net/qeth_core_mpc.c
+++ b/drivers/s390/net/qeth_core_mpc.c
@@ -222,8 +222,7 @@ struct ipa_rc_msg {
 char *qeth_get_ipa_msg(enum qeth_ipa_return_codes rc)
 {
 	int x = 0;
-	qeth_ipa_rc_msg[sizeof(qeth_ipa_rc_msg) /
-			sizeof(struct ipa_rc_msg) - 1].rc = rc;
+	qeth_ipa_rc_msg[ARRAY_SIZE(qeth_ipa_rc_msg) - 1].rc = rc;
 	while (qeth_ipa_rc_msg[x].rc != rc)
 		x++;
 	return qeth_ipa_rc_msg[x].msg;
@@ -270,9 +269,7 @@ struct ipa_cmd_names {
 char *qeth_get_ipa_cmd_name(enum qeth_ipa_cmds cmd)
 {
 	int x = 0;
-	qeth_ipa_cmd_names[
-		sizeof(qeth_ipa_cmd_names) /
-			sizeof(struct ipa_cmd_names)-1].cmd = cmd;
+	qeth_ipa_cmd_names[ARRAY_SIZE(qeth_ipa_cmd_names) - 1].cmd = cmd;
 	while (qeth_ipa_cmd_names[x].cmd != cmd)
 		x++;
 	return qeth_ipa_cmd_names[x].name;
-- 
1.7.12.4

Re: [PATCH 2/2] s390: qeth_core_mpc: Use ARRAY_SIZE instead of reimplementing its function

[Jean Delvare]

On Sat, 8 Sep 2018 18:26:28 +0800, zhong jiang wrote:
> ARRAY_SIZE has implemented its function. we prefer to use the function
> rather than the open code.
> 
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
>  drivers/s390/net/qeth_core_mpc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/s390/net/qeth_core_mpc.c b/drivers/s390/net/qeth_core_mpc.c
> index 5bcb8da..e8263de 100644
> --- a/drivers/s390/net/qeth_core_mpc.c
> +++ b/drivers/s390/net/qeth_core_mpc.c
> @@ -222,8 +222,7 @@ struct ipa_rc_msg {
>  char *qeth_get_ipa_msg(enum qeth_ipa_return_codes rc)
>  {
>  	int x = 0;
> -	qeth_ipa_rc_msg[sizeof(qeth_ipa_rc_msg) /
> -			sizeof(struct ipa_rc_msg) - 1].rc = rc;
> +	qeth_ipa_rc_msg[ARRAY_SIZE(qeth_ipa_rc_msg) - 1].rc = rc;
>  	while (qeth_ipa_rc_msg[x].rc != rc)
>  		x++;
>  	return qeth_ipa_rc_msg[x].msg;
> @@ -270,9 +269,7 @@ struct ipa_cmd_names {
>  char *qeth_get_ipa_cmd_name(enum qeth_ipa_cmds cmd)
>  {
>  	int x = 0;
> -	qeth_ipa_cmd_names[
> -		sizeof(qeth_ipa_cmd_names) /
> -			sizeof(struct ipa_cmd_names)-1].cmd = cmd;
> +	qeth_ipa_cmd_names[ARRAY_SIZE(qeth_ipa_cmd_names) - 1].cmd = cmd;
>  	while (qeth_ipa_cmd_names[x].cmd != cmd)
>  		x++;
>  	return qeth_ipa_cmd_names[x].name;

Reviewed-by: Jean Delvare <jdelvare@suse.de>

BTW, this code looks racy. It is modifying a global array member
without any locking. If there is any chance that two instances of
qeth_check_ipa_data() are running in parallel and both have unknown
command (or unknown rc), we could end up overrunning either
qeth_ipa_rc_msg[] or qeth_ipa_cmd_names[]. OK, that's unlikely to
happen in practice (I suppose unknown command and unknown rc are not
supposed to happen in the first place), but that's still bad programming
style. I'll try to come up with something better...

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH 0/2] s390: Use ARRAY_SIZE instead of reimplementing its function

[Martin Schwidefsky]

On Sat, 8 Sep 2018 18:26:26 +0800
zhong jiang <zhongjiang@huawei.com> wrote:

> I find the issue with the help of Coccinelle.
> 
> zhong jiang (2):
>   s390: vmlogrdr: Use ARRAY_SIZE instead of reimplementing its function
>   s390: qeth_core_mpc: Use ARRAY_SIZE instead of reimplementing its
>     function
> 
>  drivers/s390/char/vmlogrdr.c     | 2 +-
>  drivers/s390/net/qeth_core_mpc.c | 7 ++-----
>  2 files changed, 3 insertions(+), 6 deletions(-)

Both patches applied to linux-s390:features. Thanks.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.