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