On Thu. 26 Jun. 2025 at 01:47, Markus Elfring <Markus.Elfring@xxxxxx> wrote: > > A real quick search shows me that this ucan driver is not an isolated case. > > Here is an example: > > > > https://elixir.bootlin.com/linux/v6.16-rc3/source/drivers/media/rc/imon.c#L2137-L2148 > > Thanks that you pointed another implementation detail out from > the function “imon_find_endpoints”. What I did here was simply to look at all the users of the USB_ENDPOINT_DIR_MASK macro: https://elixir.bootlin.com/linux/v6.16-rc3/C/ident/USB_ENDPOINT_DIR_MASK and bingo, the very first user of that macro is the imon driver with a true positive. I did not check the other drivers from the list, but that is what I meant by the manual hunt: I believe that 15 minutes would be enough to quickly check all those drivers. Of course, doing it manually is a one time solution whereas adding the coccinelle script is a long term solution. Also, I am just sharing my thoughts. I am not trying to discourage you in any way, it is even the opposite: such initiatives are really nice! Even if I do not participate in these myself, I want to tell you my gratitude for your efforts! > > But it does not seem to occur so often either. So not sure what is the best: > > do a manual hunt > > Unlikely. > > I am unsure if such an aspect would become relevant for a code review > by other contributors. > > > > or write a coccinelle checker. > > I would find it more convenient to achieve corresponding adjustments > to some degree with the help of such a development tool. > I constructed scripts for the semantic patch language accordingly. > > > >> 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 > > > > Further simplification, nice :) > > > > I didn't see that last one, so glad you found what seems to be the optimal solution! > I am unsure if the check reordering would be desirable for this function implementation. Ah, you want to confirm whether usb_endpoint_dir_in(ep) && usb_endpoint_xfer_bulk(ep) is the same as usb_endpoint_xfer_bulk(ep) && usb_endpoint_dir_in(ep) ? In this case, that is OK. *Mathematically speaking* we have this equivalence: a & b <=> b & a In C it is roughly the same except if the expression has some undefined behaviour. The typical example is: foo && foo->bar Here, the short cut evaluation of the && operator will prevent the undefined behaviour to occur if foo is NULL. And so, obviously, refactoring as: foo->bar && foo would be a bug. In our case, there is no undefined behaviour on the right hand operand (I mean, if ep is NULL, the undefined behaviour will already occur on the left hand operand). So we are totally safe to reorder the operand here. Yours sincerely, Vincent Mailhol
