[PATCH 1/6] Add ancillary bus support

[Greg KH]

On Thu, Oct 01, 2020 at 09:02:12AM -0300, Jason Gunthorpe wrote:
> On Thu, Oct 01, 2020 at 01:54:02PM +0200, Greg KH wrote:
> > On Thu, Oct 01, 2020 at 08:46:08AM -0300, Jason Gunthorpe wrote:
> > > On Thu, Oct 01, 2020 at 01:01:20PM +0200, Greg KH wrote:
> > > > On Wed, Sep 30, 2020 at 03:50:46PM -0700, Dave Ertman wrote:
> > > > > +int ancillary_device_initialize(struct ancillary_device *ancildev)
> > > > > +{
> > > > > +	struct device *dev = &ancildev->dev;
> > > > > +
> > > > > +	dev->bus = &ancillary_bus_type;
> > > > > +
> > > > > +	if (WARN_ON(!dev->parent) || WARN_ON(!ancildev->name) ||
> > > > > +	    WARN_ON(!(dev->type && dev->type->release) && !dev->release))
> > > > > +		return -EINVAL;
> > > > 
> > > > You have a lot of WARN_ON() calls in this patch.  That blows up anyone
> > > > who runs with panic-on-warn, right?
> > > 
> > > AFAIK this is the standard pattern to code a "can't happen"
> > > assertion. Linus has been clear not to use BUG_ON, but to try and
> > > recover. The WARN_ON directly points to the faulty driver so it can be
> > > fixed. 
> > 
> > Printing an error and returning an error value also does the same exact
> > thing, the developer will not have a working system.
> > 
> > Please don't abuse WARN_ON() for things that should just be normal error
> > checking logic of api calls.
> 
> This is not normal error checking, it is precondition
> assertion. Something has gone badly wrong if it ever triggers.
> 
> If you don't want to use WARN_ON for assertions then when should it be
> used?
> 
> pr_err is not the same thing, it doesn't trigger reports from fuzzers.

fuzzers shouldn't be messing with device registration functions :)

just do a normal pr_err() and all is fine, again, this is like any other
in-kernel api that is trying to check for valid values being passed to
it.

thanks,

grteg k-h