lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <66uyx73trgi4m5iybuee5ka4vbulh433o4jpqc4uu4teaflaex@av7xsfwjypy5>
Date: Thu, 5 Sep 2024 16:51:48 +0200
From: Benjamin Tissoires <bentiss@...nel.org>
To: Thomas Weißschuh <linux@...ssschuh.net>
Cc: Jiri Kosina <jikos@...nel.org>, 
	Marcus Folkesson <marcus.folkesson@...il.com>, linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 13/14] HID: lg: constify fixed up report descriptor

As you can see in the b4 reply, I've now applied all of the patches but
this one. Please see below.

On Aug 28 2024, Thomas Weißschuh wrote:
> Now that the HID core can handle const report descriptors,
> constify them where possible.
> 
> Signed-off-by: Thomas Weißschuh <linux@...ssschuh.net>
> ---
>  drivers/hid/hid-lg.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
> index a9be918e2b5c..c1feeb1dd077 100644
> --- a/drivers/hid/hid-lg.c
> +++ b/drivers/hid/hid-lg.c
> @@ -58,7 +58,7 @@
>   * These descriptors remove the combined Y axis and instead report
>   * separate throttle (Y) and brake (RZ).
>   */
> -static __u8 df_rdesc_fixed[] = {
> +static const __u8 df_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),                   */
>  0x09, 0x04,         /*  Usage (Joystick),                       */
>  0xA1, 0x01,         /*  Collection (Application),               */
> @@ -124,7 +124,7 @@ static __u8 df_rdesc_fixed[] = {
>  0xC0                /*  End Collection                          */
>  };
>  
> -static __u8 dfp_rdesc_fixed[] = {
> +static const __u8 dfp_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),                   */
>  0x09, 0x04,         /*  Usage (Joystick),                       */
>  0xA1, 0x01,         /*  Collection (Application),               */
> @@ -172,7 +172,7 @@ static __u8 dfp_rdesc_fixed[] = {
>  0xC0                /*  End Collection                          */
>  };
>  
> -static __u8 fv_rdesc_fixed[] = {
> +static const __u8 fv_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),                   */
>  0x09, 0x04,         /*  Usage (Joystick),                       */
>  0xA1, 0x01,         /*  Collection (Application),               */
> @@ -239,7 +239,7 @@ static __u8 fv_rdesc_fixed[] = {
>  0xC0                /*  End Collection                          */
>  };
>  
> -static __u8 momo_rdesc_fixed[] = {
> +static const __u8 momo_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),               */
>  0x09, 0x04,         /*  Usage (Joystick),                   */
>  0xA1, 0x01,         /*  Collection (Application),           */
> @@ -285,7 +285,7 @@ static __u8 momo_rdesc_fixed[] = {
>  0xC0                /*  End Collection                      */
>  };
>  
> -static __u8 momo2_rdesc_fixed[] = {
> +static const __u8 momo2_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),               */
>  0x09, 0x04,         /*  Usage (Joystick),                   */
>  0xA1, 0x01,         /*  Collection (Application),           */
> @@ -333,7 +333,7 @@ static __u8 momo2_rdesc_fixed[] = {
>  0xC0                /*  End Collection                      */
>  };
>  
> -static __u8 ffg_rdesc_fixed[] = {
> +static const __u8 ffg_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),               */
>  0x09, 0x04,         /*  Usage (Joystik),                    */
>  0xA1, 0x01,         /*  Collection (Application),           */
> @@ -379,7 +379,7 @@ static __u8 ffg_rdesc_fixed[] = {
>  0xC0                /*  End Collection                      */
>  };
>  
> -static __u8 fg_rdesc_fixed[] = {
> +static const __u8 fg_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),               */
>  0x09, 0x04,         /*  Usage (Joystik),                    */
>  0xA1, 0x01,         /*  Collection (Application),           */
> @@ -431,6 +431,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		unsigned int *rsize)
>  {
>  	struct lg_drv_data *drv_data = hid_get_drvdata(hdev);
> +	const __u8 *ret = NULL;

Not really happy about this, usually "ret" is an int, and this makes
things slightly harder to read.

>  
>  	if ((drv_data->quirks & LG_RDESC) && *rsize >= 91 && rdesc[83] == 0x26 &&
>  			rdesc[84] == 0x8c && rdesc[85] == 0x02) {
> @@ -453,7 +454,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == FG_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Wingman Formula GP report descriptor\n");
> -			rdesc = fg_rdesc_fixed;
> +			ret = fg_rdesc_fixed;

can't you just return fg_rdesc_fixed after setting *rsize, like you did
in the other patches?

(same for all of the other branches)

>  			*rsize = sizeof(fg_rdesc_fixed);
>  		} else {
>  			hid_info(hdev,
> @@ -466,7 +467,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == FFG_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Wingman Formula Force GP report descriptor\n");
> -			rdesc = ffg_rdesc_fixed;
> +			ret = ffg_rdesc_fixed;
>  			*rsize = sizeof(ffg_rdesc_fixed);
>  		}
>  		break;
> @@ -476,7 +477,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == DF_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Driving Force report descriptor\n");
> -			rdesc = df_rdesc_fixed;
> +			ret = df_rdesc_fixed;
>  			*rsize = sizeof(df_rdesc_fixed);
>  		}
>  		break;
> @@ -485,7 +486,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == MOMO_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Momo Force (Red) report descriptor\n");
> -			rdesc = momo_rdesc_fixed;
> +			ret = momo_rdesc_fixed;
>  			*rsize = sizeof(momo_rdesc_fixed);
>  		}
>  		break;
> @@ -494,7 +495,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == MOMO2_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Momo Racing Force (Black) report descriptor\n");
> -			rdesc = momo2_rdesc_fixed;
> +			ret = momo2_rdesc_fixed;
>  			*rsize = sizeof(momo2_rdesc_fixed);
>  		}
>  		break;
> @@ -503,7 +504,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == FV_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Formula Vibration report descriptor\n");
> -			rdesc = fv_rdesc_fixed;
> +			ret = fv_rdesc_fixed;
>  			*rsize = sizeof(fv_rdesc_fixed);
>  		}
>  		break;
> @@ -512,7 +513,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == DFP_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Driving Force Pro report descriptor\n");
> -			rdesc = dfp_rdesc_fixed;
> +			ret = dfp_rdesc_fixed;
>  			*rsize = sizeof(dfp_rdesc_fixed);
>  		}
>  		break;
> @@ -529,6 +530,8 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		break;
>  	}
>  
> +	if (ret)
> +		return ret;
>  	return rdesc;
>  }
>  
> 
> -- 
> 2.46.0
> 

Cheers,
Benjamin

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ