diff mbox

HID: fix noderef.cocci warnings

Message ID OSXPR01MB0408C44F44A1B06579D0BCB7C72A0@OSXPR01MB0408.jpnprd01.prod.outlook.com (mailing list archive)
State New, archived
Headers show

Commit Message

Masaki Ota June 20, 2016, 9:45 a.m. UTC 
Hi, fengguang,

	ret = hid_hw_raw_request(hdev, U1_FEATURE_REPORT_ID, input,
-			sizeof(input), HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+			sizeof(*input), HID_FEATURE_REPORT,
+			HID_REQ_SET_REPORT);

I tested this code, but our device cannot work on it.
So, I think we should modify the code as below.

	ret = hid_hw_raw_request(hdev, U1_FEATURE_REPORT_ID, input,
			sizeof(u8)*U1_FEATURE_REPORT_LEN,
			HID_FEATURE_REPORT, HID_REQ_SET_REPORT);

Best Regards,
Masaki Ota
-----Original Message-----
From: kbuild test robot [mailto:fengguang.wu@intel.com] 
Sent: Saturday, June 18, 2016 9:13 PM
Cc: kbuild-all@01.org; linux-input@vger.kernel.org; linux-usb@vger.kernel.org; Jiri Kosina; 太田 真喜 Masaki Ota
Subject: [PATCH] HID: fix noderef.cocci warnings

drivers/hid/hid-alps.c:139:3-9: ERROR: application of sizeof to pointer
drivers/hid/hid-alps.c:148:4-10: ERROR: application of sizeof to pointer

 sizeof when applied to a pointer typed expression gives the size of  the pointer

Generated by: scripts/coccinelle/misc/noderef.cocci

CC: Masaki Ota <masaki.ota@jp.alps.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 hid-alps.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

 
 	if (read_flag) {
 		ret = hid_hw_raw_request(hdev, U1_FEATURE_REPORT_ID, readbuf,
-				sizeof(readbuf), HID_FEATURE_REPORT,
+				sizeof(*readbuf), HID_FEATURE_REPORT,
 				HID_REQ_GET_REPORT);
 
 		if (ret < 0) {
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Fengguang Wu June 20, 2016, 10:02 a.m. UTC | #1 
Hi Masaki,

On Mon, Jun 20, 2016 at 09:45:02AM +0000, Masaki Ota wrote:
> Hi, fengguang,
> 
> 	ret = hid_hw_raw_request(hdev, U1_FEATURE_REPORT_ID, input,
> -			sizeof(input), HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> +			sizeof(*input), HID_FEATURE_REPORT,
> +			HID_REQ_SET_REPORT);
> 
> I tested this code, but our device cannot work on it.
> So, I think we should modify the code as below.
> 
> 	ret = hid_hw_raw_request(hdev, U1_FEATURE_REPORT_ID, input,
> 			sizeof(u8)*U1_FEATURE_REPORT_LEN,
> 			HID_FEATURE_REPORT, HID_REQ_SET_REPORT);

OK, thank you for the fix!

Fenguang

> -----Original Message-----
> From: kbuild test robot [mailto:fengguang.wu@intel.com] 
> Sent: Saturday, June 18, 2016 9:13 PM
> Cc: kbuild-all@01.org; linux-input@vger.kernel.org; linux-usb@vger.kernel.org; Jiri Kosina; 太田 真喜 Masaki Ota
> Subject: [PATCH] HID: fix noderef.cocci warnings
> 
> drivers/hid/hid-alps.c:139:3-9: ERROR: application of sizeof to pointer
> drivers/hid/hid-alps.c:148:4-10: ERROR: application of sizeof to pointer
> 
>  sizeof when applied to a pointer typed expression gives the size of  the pointer
> 
> Generated by: scripts/coccinelle/misc/noderef.cocci
> 
> CC: Masaki Ota <masaki.ota@jp.alps.com>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> ---
> 
>  hid-alps.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> --- a/drivers/hid/hid-alps.c
> +++ b/drivers/hid/hid-alps.c
> @@ -136,7 +136,8 @@ static int u1_read_write_register(struct
>  
>  	input[7] = check_sum;
>  	ret = hid_hw_raw_request(hdev, U1_FEATURE_REPORT_ID, input,
> -			sizeof(input), HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> +			sizeof(*input), HID_FEATURE_REPORT,
> +			HID_REQ_SET_REPORT);
>  
>  	if (ret < 0) {
>  		dev_err(&hdev->dev, "failed to read command (%d)\n", ret); @@ -145,7 +146,7 @@ static int u1_read_write_register(struct
>  
>  	if (read_flag) {
>  		ret = hid_hw_raw_request(hdev, U1_FEATURE_REPORT_ID, readbuf,
> -				sizeof(readbuf), HID_FEATURE_REPORT,
> +				sizeof(*readbuf), HID_FEATURE_REPORT,
>  				HID_REQ_GET_REPORT);
>  
>  		if (ret < 0) {
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Kosina June 20, 2016, 10:40 a.m. UTC | #2 
On Mon, 20 Jun 2016, Masaki Ota wrote:

> Hi, fengguang,
> 
> 	ret = hid_hw_raw_request(hdev, U1_FEATURE_REPORT_ID, input,
> -			sizeof(input), HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> +			sizeof(*input), HID_FEATURE_REPORT,
> +			HID_REQ_SET_REPORT);

This is a wrong fix. sizeof(*input) is sizeof(u8).

> I tested this code, but our device cannot work on it.
> So, I think we should modify the code as below.
> 
> 	ret = hid_hw_raw_request(hdev, U1_FEATURE_REPORT_ID, input,
> 			sizeof(u8)*U1_FEATURE_REPORT_LEN,
> 			HID_FEATURE_REPORT, HID_REQ_SET_REPORT);

I've already applied patch that fixes this (an one more callsite with the 
same issue); I CCed you on the patch earlier today.
diff mbox

Patch

--- a/drivers/hid/hid-alps.c
+++ b/drivers/hid/hid-alps.c
@@ -136,7 +136,8 @@  static int u1_read_write_register(struct
 
 	input[7] = check_sum;
 	ret = hid_hw_raw_request(hdev, U1_FEATURE_REPORT_ID, input,
-			sizeof(input), HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+			sizeof(*input), HID_FEATURE_REPORT,
+			HID_REQ_SET_REPORT);
 
 	if (ret < 0) {
 		dev_err(&hdev->dev, "failed to read command (%d)\n", ret); @@ -145,7 +146,7 @@ static int u1_read_write_register(struct