* [PATCH 0/2] s390: Use ARRAY_SIZE instead of reimplementing its function
@ 2018-09-08 10:26 ` zhong jiang
0 siblings, 0 replies; 12+ messages in thread
From: zhong jiang @ 2018-09-08 10:26 UTC (permalink / raw)
To: schwidefsky, heiko.carstens, jwi, ubraun, jdelvare
Cc: linux-s390, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] s390: vmlogrdr: Use ARRAY_SIZE instead of reimplementing its function
2018-09-08 10:26 ` zhong jiang
@ 2018-09-08 10:26 ` zhong jiang
-1 siblings, 0 replies; 12+ messages in thread
From: zhong jiang @ 2018-09-08 10:26 UTC (permalink / raw)
To: schwidefsky, heiko.carstens, jwi, ubraun, jdelvare
Cc: linux-s390, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 1/2] s390: vmlogrdr: Use ARRAY_SIZE instead of reimplementing its function
@ 2018-09-08 10:26 ` zhong jiang
0 siblings, 0 replies; 12+ messages in thread
From: zhong jiang @ 2018-09-08 10:26 UTC (permalink / raw)
To: schwidefsky, heiko.carstens, jwi, ubraun, jdelvare
Cc: linux-s390, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] s390: vmlogrdr: Use ARRAY_SIZE instead of reimplementing its function
2018-09-08 10:26 ` zhong jiang
@ 2018-09-08 13:00 ` Jean Delvare
-1 siblings, 0 replies; 12+ messages in thread
From: Jean Delvare @ 2018-09-08 13:00 UTC (permalink / raw)
To: zhong jiang
Cc: schwidefsky, heiko.carstens, jwi, ubraun, linux-s390,
linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] s390: vmlogrdr: Use ARRAY_SIZE instead of reimplementing its function
@ 2018-09-08 13:00 ` Jean Delvare
0 siblings, 0 replies; 12+ messages in thread
From: Jean Delvare @ 2018-09-08 13:00 UTC (permalink / raw)
To: zhong jiang
Cc: schwidefsky, heiko.carstens, jwi, ubraun, linux-s390,
linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] s390: qeth_core_mpc: Use ARRAY_SIZE instead of reimplementing its function
2018-09-08 10:26 ` zhong jiang
@ 2018-09-08 10:26 ` zhong jiang
-1 siblings, 0 replies; 12+ messages in thread
From: zhong jiang @ 2018-09-08 10:26 UTC (permalink / raw)
To: schwidefsky, heiko.carstens, jwi, ubraun, jdelvare
Cc: linux-s390, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/2] s390: qeth_core_mpc: Use ARRAY_SIZE instead of reimplementing its function
@ 2018-09-08 10:26 ` zhong jiang
0 siblings, 0 replies; 12+ messages in thread
From: zhong jiang @ 2018-09-08 10:26 UTC (permalink / raw)
To: schwidefsky, heiko.carstens, jwi, ubraun, jdelvare
Cc: linux-s390, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] s390: qeth_core_mpc: Use ARRAY_SIZE instead of reimplementing its function
2018-09-08 10:26 ` zhong jiang
@ 2018-09-08 13:11 ` Jean Delvare
-1 siblings, 0 replies; 12+ messages in thread
From: Jean Delvare @ 2018-09-08 13:11 UTC (permalink / raw)
To: zhong jiang
Cc: schwidefsky, heiko.carstens, jwi, ubraun, linux-s390,
linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] s390: qeth_core_mpc: Use ARRAY_SIZE instead of reimplementing its function
@ 2018-09-08 13:11 ` Jean Delvare
0 siblings, 0 replies; 12+ messages in thread
From: Jean Delvare @ 2018-09-08 13:11 UTC (permalink / raw)
To: zhong jiang
Cc: schwidefsky, heiko.carstens, jwi, ubraun, linux-s390,
linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] s390: Use ARRAY_SIZE instead of reimplementing its function
2018-09-08 10:26 ` zhong jiang
@ 2018-09-10 6:37 ` Martin Schwidefsky
-1 siblings, 0 replies; 12+ messages in thread
From: Martin Schwidefsky @ 2018-09-10 6:37 UTC (permalink / raw)
To: zhong jiang
Cc: heiko.carstens, jwi, ubraun, jdelvare, linux-s390, linux-kernel
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] s390: Use ARRAY_SIZE instead of reimplementing its function
@ 2018-09-10 6:37 ` Martin Schwidefsky
0 siblings, 0 replies; 12+ messages in thread
From: Martin Schwidefsky @ 2018-09-10 6:37 UTC (permalink / raw)
To: zhong jiang
Cc: heiko.carstens, jwi, ubraun, jdelvare, linux-s390, linux-kernel
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.
^ permalink raw reply [flat|nested] 12+ messages in thread