On Tuesday 29 July 2008, Mauro Carvalho Chehab wrote:
On Tue, 29 Jul 2008 00:22:52 +0200
Jiri Slaby jirislaby@gmail.com wrote:
On 07/29/2008 12:16 AM, Gregor Jasny wrote:
ioctl(3, VIDIOC_REQBUFS or VT_DISALLOCATE, 0x7fffbfda0060) = 2
Huh? Something evils seems to be going on in V4L2 land. I've spotted the following lines in videobuf-core.c:videobuf_reqbufs
req->count = retval;
done: mutex_unlock(&q->vb_lock); return retval;
That would explain the retval '2'. It seems a retval = 0; statement is missing here for the success case.
Actually positive ioctl retval used to be often considered as OK in the past (and this approach is still used in few char drivers).
But according to v4l docco, it isn't permitted here. Anyway I wouldn't place it in videobuf-core.c, but in vivi code; letting this decision on Mauro (CCed) ;).
This is what videobuf-core do, if success:
req->count = retval;
done: mutex_unlock(&q->vb_lock); return retval;
So, it returns the number of buffers that were really allocated. It is too late to change this, since this behaviour is very old. If the V4L2 API spec is different, we should fix at the spec, not at the driver.
I'm not sure to agree with that. The spec clearly states that 0 is returned on success and -1 on error. Since applications don't choke with the currently buggy videobuf-core implementation, they must all be check against -1, or checking for a negative error code. So returning 0 shouldn't break any application, except if it relies on the ioctl returning the number of buffers, which is not documented anywhere.
IMO, the library patch proposed should be applied. All error checks should test for values lower than zero, since positive values don't indicate errors.
Best regards,
Laurent Pinchart