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: <0768a008-d4a9-41ec-bc47-1e7c63362296@web.de>
Date: Wed, 25 Jun 2025 07:47:39 +0200
From: Markus Elfring <Markus.Elfring@....de>
To: Vincent Mailhol <mailhol.vincent@...adoo.fr>, linux-can@...r.kernel.org
Cc: LKML <linux-kernel@...r.kernel.org>, kernel-janitors@...r.kernel.org,
 Chen Ni <nichen@...as.ac.cn>, Marc Kleine-Budde <mkl@...gutronix.de>
Subject: Re: [PATCH] can: ucan: Use usb_endpoint_type() rather than
 duplicating its implementation

>>> it seems that Coccinelle missed many simplifications.
>> Would such software transformations become better supported anyhow?
> Maybe?
> 
> I am not involved in the development of Coccinelle and thus, I don't have an answer.

This can be fine (in principle).


> Nor do I have the time to read and understand the Coccinelle source code
> to which you pointed me to.

I hope that more users can become interested in presented information.
I would appreciate if knowledge representations can be improved also for
better automatic data processing and corresponding transformations.


> My stance is that such static analyzers should never be trusted 100%.

This is generally fine.


> The output is more an indicator.

This is usual.


>                       And in this present case, a quick review made it very
> clear that Coccinelle saw one simplification but missed two other ones.

Would you find another source code adjustment (like the following) more appropriate?


diff --git a/drivers/net/can/usb/ucan.c b/drivers/net/can/usb/ucan.c
index 07406daf7c88..6c6cee3895c6 100644
--- a/drivers/net/can/usb/ucan.c
+++ b/drivers/net/can/usb/ucan.c
@@ -1352,17 +1352,12 @@ static int ucan_probe(struct usb_interface *intf,
 	for (i = 0; i < iface_desc->desc.bNumEndpoints; i++) {
 		ep = &iface_desc->endpoint[i].desc;
 
-		if (((ep->bEndpointAddress & USB_ENDPOINT_DIR_MASK) != 0) &&
-		    ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
-		     USB_ENDPOINT_XFER_BULK)) {
+		if (usb_endpoint_dir_in(ep) && usb_endpoint_xfer_bulk(ep)) {
 			/* In Endpoint */
 			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)) {
+		} else if (usb_endpoint_dir_out(ep) && usb_endpoint_xfer_bulk(ep)) {
 			/* Out Endpoint */
 			out_ep_addr = ep->bEndpointAddress;
 			out_ep_addr &= USB_ENDPOINT_NUMBER_MASK;


Can the functions “usb_endpoint_is_bulk_in” and “usb_endpoint_is_bulk_out”
be applied here?
https://elixir.bootlin.com/linux/v6.16-rc3/source/include/uapi/linux/usb/ch9.h#L572-L595

Regards,
Markus