linux-kernel - Re: [PATCH] can: ucan: Use usb_endpoint_type() rather than duplicating its implementation

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: <bf5442d1-34e0-495f-8a56-4e47f53ca4ad@wanadoo.fr>
Date: Wed, 25 Jun 2025 02:29:56 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: Markus Elfring <Markus.Elfring@....de>
Cc: LKML <linux-kernel@...r.kernel.org>, kernel-janitors@...r.kernel.org,
 Chen Ni <nichen@...as.ac.cn>, linux-can@...r.kernel.org,
 Marc Kleine-Budde <mkl@...gutronix.de>
Subject: Re: [PATCH] can: ucan: Use usb_endpoint_type() rather than
 duplicating its implementation

Hi Markus,

On 25/06/2025 at 02:12, Markus Elfring wrote:
> From: Markus Elfring <elfring@...rs.sourceforge.net>
> Date: Tue, 24 Jun 2025 19:05:04 +0200
> 
> Reuse existing functionality from usb_endpoint_type() instead of keeping
> duplicate source code.
> 
> The source code was transformed by using the Coccinelle software.

Looking at the helpers in linux/usb/ch9.h, it seems that Coccinelle missed many
simplifications.

> Signed-off-by: Markus Elfring <elfring@...rs.sourceforge.net>

Can you use the same e-mail address for signing and for sending the patch?

> ---
>  drivers/net/can/usb/ucan.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/can/usb/ucan.c b/drivers/net/can/usb/ucan.c
> index 07406daf7c88..3c8dad8bcca4 100644
> --- a/drivers/net/can/usb/ucan.c
> +++ b/drivers/net/can/usb/ucan.c
> @@ -1353,16 +1353,14 @@ static int ucan_probe(struct usb_interface *intf,
>  		ep = &iface_desc->endpoint[i].desc;
>  
>  		if (((ep->bEndpointAddress & USB_ENDPOINT_DIR_MASK) != 0) &&
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is:

	usb_endpoint_dir_in(ep)

> -		    ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
> -		     USB_ENDPOINT_XFER_BULK)) {
> +		    usb_endpoint_type(ep) == USB_ENDPOINT_XFER_BULK) {
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is:

	usb_endpoint_xfer_bulk(ep)

>  			/* In Endpoint */
After changing to usb_endpoint_dir_in(ep), this comment can be removed.

>  			in_ep_addr = ep->bEndpointAddress;
>  			in_ep_addr &= USB_ENDPOINT_NUMBER_MASK;
>  			in_ep_size = le16_to_cpu(ep->wMaxPacketSize);
>  		} else if (((ep->bEndpointAddress & USB_ENDPOINT_DIR_MASK) ==
>  			    0) &&
> -			   ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
> -			    USB_ENDPOINT_XFER_BULK)) {
> +			   usb_endpoint_type(ep) == USB_ENDPOINT_XFER_BULK) {

Similarly as above use:

	usb_endpoint_dir_out(ep)

and:

	usb_endpoint_xfer_bulk(ep)

>  			/* Out Endpoint */
>  			out_ep_addr = ep->bEndpointAddress;
>  			out_ep_addr &= USB_ENDPOINT_NUMBER_MASK;


Yours sincerely,
Vincent Mailhol