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.
Jason
