Re: [PATCH 3/3 v5] USB: Fix device driver race

[Alan Stern <stern@rowland.harvard.edu>]

On Sun, Jul 26, 2020 at 10:36:55AM +0200, Greg Kroah-Hartman wrote:
> On Sat, Jul 25, 2020 at 03:57:07PM -0400, Alan Stern wrote:
> > On Sat, Jul 25, 2020 at 05:24:20PM +0200, Bastien Nocera wrote:
> > > On Sat, 2020-07-25 at 10:59 -0400, Alan Stern wrote:
> > > <snip>
> > > > > +	udev = to_usb_device(dev);
> > > > > +	if (usb_device_match_id(udev, new_udriver->id_table) == NULL &&
> > > > > +	    (!new_udriver->match || new_udriver->match(udev) != 0))
> > > > > +		return 0;
> > > > > +
> > > > > +	(void)!device_reprobe(dev);
> > > > 
> > > > What's that '!' doing hiding in there?  It doesn't affect the final 
> > > > outcome, but it sure looks weird -- if people notice it at all.
> > > 
> > > It's how we stop gcc from complaining about the warn_unused_result
> > > attribute on device_reprobe()... (void) is enough with clang, but not
> > > with gcc.
> > 
> > Hmmm.  Maybe this is an indication that device_reprobe() doesn't really 
> > need to be __must_check.
> > 
> > Greg, do you know why it's annotated this way?
> 
> Because you really should pass up the return value if an error happens
> here.  Why do we think it is safe to ignore?
> 
> And that "(void)!" is not ok, if the annotation is safe to ignore, then
> we need to remove the annotation, don't work around stuff like this
> without at the very least, a comment saying why it is ok.

I suppose Bastien could log an error message at that point.  There isn't 
much else to do.

Alan Stern