On Fri, Jul 24, 2020 at 6:27 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Jul 24, 2020 at 01:32:40PM +0300, Andy Shevchenko wrote:
> > On Fri, Jul 24, 2020 at 12:03 PM Bastien Nocera <hadess@xxxxxxxxxx> wrote:
...
> > What about
> >
> > static bool is_dev_usb_generic_driver(dev)
> > {
> > struct usb_device_driver *udd = dev->driver ?
> > to_usb_device_driver(dev->driver) : NULL;
> >
> > return udd == &usb_generic_driver;
> > }
> >
> > if (!is_dev_usb_generic_driver)
> > return 0;
> >
> > ?
>
> Why would you want to add more lines of code to do the same thing?
> Abstraction is fine up to point, but excessive abstraction only makes
> things more difficult.
At least to put into ternary will actually reduce LOCs in the original
code (if we drop helper).
The idea of a helper comes to mind that somebody else might reuse
(__check_usb_generic()?).
...
> The logic is wrong. If usb_device_match_id() returns NULL then we want
> to go ahead with the second test, not give up right away. But this
> could be written as:
Yes, sorry I didn't think well here.
>
> if (usb_device_match_id(udev, new_udriver->id_table) == NULL &&
> (!new_udriver->match || new_udriver->match(udev) != 0))
> return 0;
>
> device_reprobe(dev);
>
> This would make the overall flow of the routine more uniform.
Yes, my intention was to make all error cases go first.
--
With Best Regards,
Andy Shevchenko
