[FFmpeg-devel] [PATCH] libavdevice/v4l2: fix of crash caused by assert

[Giorgio Vazzana]

2014-09-01 22:46 GMT+02:00 Reimar Döffinger <Reimar.Doeffinger at gmx.de>:
> On Mon, Sep 01, 2014 at 10:33:15PM +0200, Giorgio Vazzana wrote:
>> +static int enqueue_buffer(struct video_data *s, struct v4l2_buffer *buf)
>> +{
>> +    int res = 0;
>> +
>> +    if (v4l2_ioctl(s->fd, VIDIOC_QBUF, buf) < 0) {
>> +        res = AVERROR(errno);
>> +        av_log(NULL, AV_LOG_ERROR, "ioctl(VIDIOC_QBUF): %s\n", av_err2str(res));
>> +    } else {
>> +        avpriv_atomic_int_add_and_fetch(&s->buffers_queued, 1);
>> +    }
>> +
>> +    return res;
>
> Nit: I'd prefer something simpler that avoids cluttering the normal path
> because of error handling like:
>
>> +    if (v4l2_ioctl(s->fd, VIDIOC_QBUF, buf) < 0) {
>> +        av_log(NULL, AV_LOG_ERROR, "ioctl(VIDIOC_QBUF): %s\n", av_err2str(res));
>> +        return AVERROR(errno);

If we call av_log(), errno is not guaranteed to be the same after the
call. Also, av_err2str() needs AVERROR(errno).

>> +    }
>> +    avpriv_atomic_int_add_and_fetch(&s->buffers_queued, 1);
>> +    return 0;
>
>
>
>
>>  static void mmap_release_buffer(void *opaque, uint8_t *data)
>
> I haven't checked, but I suspect this function could be made simpler
> and more likely to not re-introduce these bugs by using for the
> error paths goto.
> Admittedly that doesn't belong in this patch anyway though.

OK, we'll fix this in another patch then.