Anton Khirnov:
> fd is local to the loop iteration, it is better to store it on stack
> than modify the context.
> ---
> libavdevice/v4l2.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
> index b5997fba33..777867db86 100644
> --- a/libavdevice/v4l2.c
> +++ b/libavdevice/v4l2.c
> @@ -1033,16 +1033,17 @@ static int v4l2_get_device_list(AVFormatContext *ctx,
> AVDeviceInfoList *device_l
> return ret;
> }
> while ((entry = readdir(dir))) {
> + int fd = -1;
> char device_name[256];
>
> if (!v4l2_is_v4l_dev(entry->d_name))
> continue;
>
> snprintf(device_name, sizeof(device_name), "/dev/%s", entry->d_name);
> - if ((s->fd = device_open(ctx, device_name)) < 0)
> + if ((fd = device_open(ctx, device_name)) < 0)
> continue;
>
> - if (v4l2_ioctl(s->fd, VIDIOC_QUERYCAP, &cap) < 0) {
> + if (v4l2_ioctl(fd, VIDIOC_QUERYCAP, &cap) < 0) {
> ret = AVERROR(errno);
> av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_QUERYCAP): %s\n",
> av_err2str(ret));
> goto fail;
> @@ -1064,8 +1065,7 @@ static int v4l2_get_device_list(AVFormatContext *ctx,
> AVDeviceInfoList *device_l
> &device_list->nb_devices, device))
> < 0)
> goto fail;
>
> - v4l2_close(s->fd);
> - s->fd = -1;
> + v4l2_close(fd);
> continue;
>
> fail:
> @@ -1074,9 +1074,8 @@ static int v4l2_get_device_list(AVFormatContext *ctx,
> AVDeviceInfoList *device_l
> av_freep(&device->device_description);
> av_freep(&device);
> }
> - if (s->fd >= 0)
> - v4l2_close(s->fd);
> - s->fd = -1;
> + if (fd >= 0)
This checks is unnecessary: One only goes to fail after the device has
been successfully opened.
> + v4l2_close(fd);
> break;
> }
> closedir(dir);
>
1. Commit title is wrong: This is lavd.
2. Given that there is no equivalent of avformat_alloc_output_context2()
one has to use avformat_open_input() to use avdevice_list_devices().
This means that it is possible for read_header to have been called
before v4l2_get_device_list() and in this case this commit fixes a file
descriptor leak; it also avoids closing an invalid (-1) file descriptor
v4l2_read_close(). This should be mentioned in the commit message; in
fact, this is the main advantage of this commit.
(I have not checked what would have happened had I actually used this
context with the invalid file descriptor afterwards to read data.)
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
