Re: [PATCH 9/9] videobuf2-core: Change vb2_queue_init return type to void

Hi Jon,

Thanks for your answers, I really appreciate it.

On Tue, Aug 28, 2012 at 1:55 PM, Jonathan Corbet [off-list ref] wrote:

On Sun, 26 Aug 2012 19:59:40 -0300
Ezequiel Garcia [off-list ref] wrote:

quoted

1.
Why do we need to check for all these conditions in the first place?
There are many other functions relying on "struct vb2_queue *q"
not being null (almost all of them) and we don't check for it.
What makes vb2_queue_init() so special that we need to check for it?

There are plenty of developers who would argue for the removal of the
BUG_ON(!q) line regardless, since the kernel will quickly crash shortly
thereafter.  I'm a bit less convinced; there are attackers who are very
good at exploiting null pointer dereferences, and some systems still allow
the low part of the address space to be mapped.

In general, IMO, checks for consistency make sense; it's nice if the
kernel can *tell* you that something is wrong.

What's a mistake is the BUG_ON; that should really only be used in places
where things simply cannot continue.  In this case, the initialization can
be failed, the V4L2 device will likely be unavailable, but everything else
can continue as normal.  -EINVAL is the right response here.

I see your point.

What I really can't seem to understand is why we should have a check
at vb2_queue_init() but not at vb2_get_drv_priv(), just to pick one.

Thanks a lot!
Ezequiel.