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 ...
For the unregister() paths a WARN() would be fair. Again,
any time you're tempted to use BUG() or BUG_ON(), you need
to re-think. It's hardly ever the right thing to do. Just
report the error and continue; callers should check, and
clean up if something went wrong.
> +EXPORT_SYMBOL(v4l2_device_register);
This may be a nit, but I wonder why not EXPORT_SYMBOL_GPL,
which seems to be more correct in this case.
Another quasi-style point: v4l2-device.s and v4l-subdev.c
are so small, and conceptually related, that I'd be tempted
to have one file not two. Ditto their headers.
- Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
