2014-12-23 15:27:57

by Vaishali Thakkar

Subject: [PATCH] NFC: port100: Introduce the use of function put_unaligned_le16

This patch introduces the use of function put_unaligned_le16.

This is done using Coccinelle and semantic patch used is as follows:

@a@
typedef u16, __le16;
{u16,__le16} e16;
identifier tmp;
expression ptr;
expression y,e;
type T;
@@

- tmp = cpu_to_le16(y);

<+... when != tmp
(
- memcpy(ptr, (T)&tmp, \(2\|sizeof(u16)\|sizeof(__le16)\|sizeof(e16)\));
+ put_unaligned_le16(y,ptr);
|
- memcpy(ptr, (T)&tmp, ...);
+ put_unaligned_le16(y,ptr);
)
...+>
? tmp = e

@@ type T; identifier a.tmp; @@

- T tmp;
...when != tmp

Signed-off-by: Vaishali Thakkar <[email protected]>
---
drivers/nfc/port100.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/nfc/port100.c b/drivers/nfc/port100.c
index 4ac4d31..f7cbca8 100644
--- a/drivers/nfc/port100.c
+++ b/drivers/nfc/port100.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <linux/usb.h>
#include <net/nfc/digital.h>
+#include <linux/unaligned/access_ok.h>

#define VERSION "0.1"

@@ -1136,7 +1137,6 @@ static int port100_in_send_cmd(struct nfc_digital_dev *ddev,
{
struct port100 *dev = nfc_digital_get_drvdata(ddev);
struct port100_cb_arg *cb_arg;
- __le16 timeout;

cb_arg = kzalloc(sizeof(struct port100_cb_arg), GFP_KERNEL);
if (!cb_arg)
@@ -1145,9 +1145,7 @@ static int port100_in_send_cmd(struct nfc_digital_dev *ddev,
cb_arg->complete_cb = cb;
cb_arg->complete_arg = arg;

- timeout = cpu_to_le16(_timeout * 10);
-
- memcpy(skb_push(skb, sizeof(__le16)), &timeout, sizeof(__le16));
+ put_unaligned_le16(_timeout * 10, skb_push(skb, sizeof(__le16)));

return port100_send_cmd_async(dev, PORT100_CMD_IN_COMM_RF, skb,
port100_in_comm_rf_complete, cb_arg);
--
1.9.1

2014-12-23 19:24:09

by Arend van Spriel

Subject: Re: [PATCH] NFC: port100: Introduce the use of function put_unaligned_le16

On 12/23/14 16:27, Vaishali Thakkar wrote:
> This patch introduces the use of function put_unaligned_le16.
>
> This is done using Coccinelle and semantic patch used is as follows:
>
> @a@
> typedef u16, __le16;
> {u16,__le16} e16;
> identifier tmp;
> expression ptr;
> expression y,e;
> type T;
> @@
>
> - tmp = cpu_to_le16(y);
>
> <+... when != tmp
> (
> - memcpy(ptr, (T)&tmp, \(2\|sizeof(u16)\|sizeof(__le16)\|sizeof(e16)\));
> + put_unaligned_le16(y,ptr);
> |
> - memcpy(ptr, (T)&tmp, ...);
> + put_unaligned_le16(y,ptr);
> )
> ...+>
> ? tmp = e
>
> @@ type T; identifier a.tmp; @@
>
> - T tmp;
> ...when != tmp
>
> Signed-off-by: Vaishali Thakkar<[email protected]>
> ---
> drivers/nfc/port100.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nfc/port100.c b/drivers/nfc/port100.c
> index 4ac4d31..f7cbca8 100644
> --- a/drivers/nfc/port100.c
> +++ b/drivers/nfc/port100.c
> @@ -18,6 +18,7 @@
> #include<linux/module.h>
> #include<linux/usb.h>
> #include<net/nfc/digital.h>
> +#include<linux/unaligned/access_ok.h>

It is incorrect to use this header file as was pointed out by Johannes
to me in a brcmfmac driver patch [1]. Unless this driver is architecture
specific.

Regards,
Arend

[1]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a1d69c60c44

> #define VERSION "0.1"
>
> @@ -1136,7 +1137,6 @@ static int port100_in_send_cmd(struct nfc_digital_dev *ddev,
> {
> struct port100 *dev = nfc_digital_get_drvdata(ddev);
> struct port100_cb_arg *cb_arg;
> - __le16 timeout;
>
> cb_arg = kzalloc(sizeof(struct port100_cb_arg), GFP_KERNEL);
> if (!cb_arg)
> @@ -1145,9 +1145,7 @@ static int port100_in_send_cmd(struct nfc_digital_dev *ddev,
> cb_arg->complete_cb = cb;
> cb_arg->complete_arg = arg;
>
> - timeout = cpu_to_le16(_timeout * 10);
> -
> - memcpy(skb_push(skb, sizeof(__le16)),&timeout, sizeof(__le16));
> + put_unaligned_le16(_timeout * 10, skb_push(skb, sizeof(__le16)));
>
> return port100_send_cmd_async(dev, PORT100_CMD_IN_COMM_RF, skb,
> port100_in_comm_rf_complete, cb_arg);

2014-12-24 02:35:25

by Vaishali Thakkar

Subject: Re: [PATCH] NFC: port100: Introduce the use of function put_unaligned_le16

On Wed, Dec 24, 2014 at 12:54 AM, Arend van Spriel <[email protected]> wrote:
> On 12/23/14 16:27, Vaishali Thakkar wrote:
>>
>> This patch introduces the use of function put_unaligned_le16.
>>
>> This is done using Coccinelle and semantic patch used is as follows:
>>
>> @a@
>> typedef u16, __le16;
>> {u16,__le16} e16;
>> identifier tmp;
>> expression ptr;
>> expression y,e;
>> type T;
>> @@
>>
>> - tmp = cpu_to_le16(y);
>>
>> <+... when != tmp
>> (
>> - memcpy(ptr, (T)&tmp, \(2\|sizeof(u16)\|sizeof(__le16)\|sizeof(e16)\));
>> + put_unaligned_le16(y,ptr);
>> |
>> - memcpy(ptr, (T)&tmp, ...);
>> + put_unaligned_le16(y,ptr);
>> )
>> ...+>
>> ? tmp = e
>>
>> @@ type T; identifier a.tmp; @@
>>
>> - T tmp;
>> ...when != tmp
>>
>> Signed-off-by: Vaishali Thakkar<[email protected]>
>> ---
>> drivers/nfc/port100.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/nfc/port100.c b/drivers/nfc/port100.c
>> index 4ac4d31..f7cbca8 100644
>> --- a/drivers/nfc/port100.c
>> +++ b/drivers/nfc/port100.c
>> @@ -18,6 +18,7 @@
>> #include<linux/module.h>
>> #include<linux/usb.h>
>> #include<net/nfc/digital.h>
>> +#include<linux/unaligned/access_ok.h>
>
>
> It is incorrect to use this header file as was pointed out by Johannes to me
> in a brcmfmac driver patch [1]. Unless this driver is architecture specific.
>
> Regards,
> Arend
>
> [1]
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a1d69c60c44

Ok. Thanks for pointing out. I didn't know this. So, I guess using
<asm/unaligned.h> could make sense. Should I include this??

>> #define VERSION "0.1"
>>
>> @@ -1136,7 +1137,6 @@ static int port100_in_send_cmd(struct
>> nfc_digital_dev *ddev,
>> {
>> struct port100 *dev = nfc_digital_get_drvdata(ddev);
>> struct port100_cb_arg *cb_arg;
>> - __le16 timeout;
>>
>> cb_arg = kzalloc(sizeof(struct port100_cb_arg), GFP_KERNEL);
>> if (!cb_arg)
>> @@ -1145,9 +1145,7 @@ static int port100_in_send_cmd(struct
>> nfc_digital_dev *ddev,
>> cb_arg->complete_cb = cb;
>> cb_arg->complete_arg = arg;
>>
>> - timeout = cpu_to_le16(_timeout * 10);
>> -
>> - memcpy(skb_push(skb, sizeof(__le16)),&timeout, sizeof(__le16));
>> + put_unaligned_le16(_timeout * 10, skb_push(skb, sizeof(__le16)));
>>
>> return port100_send_cmd_async(dev, PORT100_CMD_IN_COMM_RF, skb,
>> port100_in_comm_rf_complete,
>> cb_arg);
>
>



--
Vaishali

2014-12-24 09:10:48

by Arend van Spriel

Subject: Re: [PATCH] NFC: port100: Introduce the use of function put_unaligned_le16

On 12/24/14 03:35, Vaishali Thakkar wrote:
> On Wed, Dec 24, 2014 at 12:54 AM, Arend van Spriel<[email protected]> wrote:
>> On 12/23/14 16:27, Vaishali Thakkar wrote:
>>>
>>> This patch introduces the use of function put_unaligned_le16.
>>>
>>> This is done using Coccinelle and semantic patch used is as follows:
>>>
>>> @a@
>>> typedef u16, __le16;
>>> {u16,__le16} e16;
>>> identifier tmp;
>>> expression ptr;
>>> expression y,e;
>>> type T;
>>> @@
>>>
>>> - tmp = cpu_to_le16(y);
>>>
>>> <+... when != tmp
>>> (
>>> - memcpy(ptr, (T)&tmp, \(2\|sizeof(u16)\|sizeof(__le16)\|sizeof(e16)\));
>>> + put_unaligned_le16(y,ptr);
>>> |
>>> - memcpy(ptr, (T)&tmp, ...);
>>> + put_unaligned_le16(y,ptr);
>>> )
>>> ...+>
>>> ? tmp = e
>>>
>>> @@ type T; identifier a.tmp; @@
>>>
>>> - T tmp;
>>> ...when != tmp
>>>
>>> Signed-off-by: Vaishali Thakkar<[email protected]>
>>> ---
>>> drivers/nfc/port100.c | 6 ++----
>>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/nfc/port100.c b/drivers/nfc/port100.c
>>> index 4ac4d31..f7cbca8 100644
>>> --- a/drivers/nfc/port100.c
>>> +++ b/drivers/nfc/port100.c
>>> @@ -18,6 +18,7 @@
>>> #include<linux/module.h>
>>> #include<linux/usb.h>
>>> #include<net/nfc/digital.h>
>>> +#include<linux/unaligned/access_ok.h>
>>
>>
>> It is incorrect to use this header file as was pointed out by Johannes to me
>> in a brcmfmac driver patch [1]. Unless this driver is architecture specific.
>>
>> Regards,
>> Arend
>>
>> [1]
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a1d69c60c44
>
> Ok. Thanks for pointing out. I didn't know this. So, I guess using
> <asm/unaligned.h> could make sense. Should I include this??

That's the one.

Regards,
Arend

>>> #define VERSION "0.1"
>>>
>>> @@ -1136,7 +1137,6 @@ static int port100_in_send_cmd(struct
>>> nfc_digital_dev *ddev,
>>> {
>>> struct port100 *dev = nfc_digital_get_drvdata(ddev);
>>> struct port100_cb_arg *cb_arg;
>>> - __le16 timeout;
>>>
>>> cb_arg = kzalloc(sizeof(struct port100_cb_arg), GFP_KERNEL);
>>> if (!cb_arg)
>>> @@ -1145,9 +1145,7 @@ static int port100_in_send_cmd(struct
>>> nfc_digital_dev *ddev,
>>> cb_arg->complete_cb = cb;
>>> cb_arg->complete_arg = arg;
>>>
>>> - timeout = cpu_to_le16(_timeout * 10);
>>> -
>>> - memcpy(skb_push(skb, sizeof(__le16)),&timeout, sizeof(__le16));
>>> + put_unaligned_le16(_timeout * 10, skb_push(skb, sizeof(__le16)));
>>>
>>> return port100_send_cmd_async(dev, PORT100_CMD_IN_COMM_RF, skb,
>>> port100_in_comm_rf_complete,
>>> cb_arg);
>>
>>
>
>
>