Re: [PATCH 1/6] Add ancillary bus support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

thanks,

greg k-h



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux