Re: [PATCH v2] v4l2_device/v4l2_subdev: final (?) version

[David Brownell <david-b@pacbell.net>]

On Saturday 29 November 2008, Hans Verkuil wrote:
> > > +void v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev)
> > > +{ 
> > > +       BUG_ON(!dev || !v4l2_dev || dev_get_drvdata(dev));
> >
> > Ouch.  Better to return -EINVAL, like most register() calls,
> > than *ever* use a BUG_ON() for bad parameters.  Same applies
> > every other place you use BUG_ON, from a quick scan ...
> 
> Are there some documented guidelines on when to use BUG_ON?

Maybe there should be.  I know I've seen flames from Linus on
the topic.  Basically, treat it like a panic() where the system
must stop operation lest it catch fire or scribble all over the
(not-backed-up) disk ... if the system can keep running sanely,
then BUG() and friends are inappropriate.


> I see it used in other places in this way.

I tend to submit patches fixing bugs like that, when I have time.


> My reasoning was that returning an  
> error makes sense if external causes can result in an error, but this 
> test is more the equivalent of an assert(), i.e. catching a programming 
> bug early.

In which case a WARN() is better.  But in most cases I wouldn't
even do that.  The kernel's design center is closer to "run
robustly" than "make developers' lives easier".  Programmers
who don't check return values for critical operations like
registering core resources deserve what they get.  And if you
want to nudge them, the __must_check annotation helps catch
such goofage even earlier:  compile time, not run time.

- Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html