Re: [PATCH 1/2] Input-evdev: Use kmalloc_array() in evdev_handle_get_val()

[Dmitry Torokhov]

On Sat, Sep 24, 2016 at 4:08 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Sat, 24 Sep 2016 12:42:45 +0200
>
> * A multiplication for the size determination of a memory allocation
>   indicated that an array data structure should be processed.
>   Thus use the corresponding function "kmalloc_array".
>
>   This issue was detected by using the Coccinelle software.
>
> * Replace the specification of a data type by a pointer dereference
>   to make the corresponding size determination a bit safer according to
>   the Linux coding style convention.
>
> * Delete the local variable "len" which became unnecessary with
>   this refactoring.

So we have to multiply twice now, once in kmalloc_array, the second
time in memcpy(). No, thank you.

Also, please note that we do not really treat the allocated "mem" as
an array, but rather area of memory that holds all bits that we need
to transfer, and so I consider using kmalloc_array() actually wrong
here.

Please do not blindly follow checkpatch and coccinelle suggestions.
They are just that: suggestions and not hared rules.

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] Input-evdev: Use kmalloc_array() in evdev_handle_get_val()

[Dmitry Torokhov]

On Sat, Sep 24, 2016 at 4:08 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Sat, 24 Sep 2016 12:42:45 +0200
>
> * A multiplication for the size determination of a memory allocation
>   indicated that an array data structure should be processed.
>   Thus use the corresponding function "kmalloc_array".
>
>   This issue was detected by using the Coccinelle software.
>
> * Replace the specification of a data type by a pointer dereference
>   to make the corresponding size determination a bit safer according to
>   the Linux coding style convention.
>
> * Delete the local variable "len" which became unnecessary with
>   this refactoring.

So we have to multiply twice now, once in kmalloc_array, the second
time in memcpy(). No, thank you.

Also, please note that we do not really treat the allocated "mem" as
an array, but rather area of memory that holds all bits that we need
to transfer, and so I consider using kmalloc_array() actually wrong
here.

Please do not blindly follow checkpatch and coccinelle suggestions.
They are just that: suggestions and not hared rules.

Thanks.

-- 
Dmitry

[PATCH 1/2] Input-evdev: Use kmalloc_array() in evdev_handle_get_val()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 24 Sep 2016 12:42:45 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

* Delete the local variable "len" which became unnecessary with
  this refactoring.

Signed-off-by: Markus Elfring 
---
 drivers/input/evdev.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index e9ae3d5..83fcfd6 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -919,18 +919,14 @@ static int evdev_handle_get_val(struct evdev_client 
*client,
 {
int ret;
unsigned long *mem;
-   size_t len;
 
-   len = BITS_TO_LONGS(maxbit) * sizeof(unsigned long);
-   mem = kmalloc(len, GFP_KERNEL);
+   mem = kmalloc_array(BITS_TO_LONGS(maxbit), sizeof(*mem), GFP_KERNEL);
if (!mem)
return -ENOMEM;
 
spin_lock_irq(>event_lock);
spin_lock(>buffer_lock);
-
-   memcpy(mem, bits, len);
-
+   memcpy(mem, bits, sizeof(*mem) * BITS_TO_LONGS(maxbit));
spin_unlock(>event_lock);
 
__evdev_flush_queue(client, type);
-- 
2.10.0

[PATCH 1/2] Input-evdev: Use kmalloc_array() in evdev_handle_get_val()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 24 Sep 2016 12:42:45 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

* Delete the local variable "len" which became unnecessary with
  this refactoring.

Signed-off-by: Markus Elfring 
---
 drivers/input/evdev.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index e9ae3d5..83fcfd6 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -919,18 +919,14 @@ static int evdev_handle_get_val(struct evdev_client 
*client,
 {
int ret;
unsigned long *mem;
-   size_t len;
 
-   len = BITS_TO_LONGS(maxbit) * sizeof(unsigned long);
-   mem = kmalloc(len, GFP_KERNEL);
+   mem = kmalloc_array(BITS_TO_LONGS(maxbit), sizeof(*mem), GFP_KERNEL);
if (!mem)
return -ENOMEM;
 
spin_lock_irq(>event_lock);
spin_lock(>buffer_lock);
-
-   memcpy(mem, bits, len);
-
+   memcpy(mem, bits, sizeof(*mem) * BITS_TO_LONGS(maxbit));
spin_unlock(>event_lock);
 
__evdev_flush_queue(client, type);
-- 
2.10.0