Re: [V4l2-library] Messed up syscall return value

On Tuesday 29 July 2008, Mauro Carvalho Chehab wrote:
> On Tue, 29 Jul 2008 00:22:52 +0200
>
> Jiri Slaby <[email protected]> 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

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list