[libcamera-devel] [PATCH 03/14] libcamera: v4l2_videodevice: Move FD duplication in V4L2Device::Open

[Jacopo Mondi]

Hello Xavier,

On Thu, Sep 08, 2022 at 08:48:39PM +0200, Xavier Roumegue via libcamera-devel wrote:
> This ensures that the V4L2Device controls attributes have been enumerated
> in its open() method in case of file descriptor duplication.
>

Do I get this right or the actual issue is the controls are not
enumerated for M2M devices because V4L2Device::open() (and consequentially
V4L2Device::listControls()) is never called for them ?

If that's the case, I would squash 2 and 3 and clarify this in the
commit message.

> Signed-off-by: Xavier Roumegue <xavier.roumegue at oss.nxp.com>
> ---
>  src/libcamera/v4l2_videodevice.cpp | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 955e1508..d0427fef 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -661,17 +661,9 @@ int V4L2VideoDevice::open(SharedFD handle, enum v4l2_buf_type type)
>  {
>  	int ret;
>
> -	UniqueFD newFd = handle.dup();
> -	if (!newFd.isValid()) {
> -		ret = -errno;
> -		LOG(V4L2, Error) << "Failed to duplicate file handle: "
> -				 << strerror(-ret);
> -		return ret;
> -	}
> -
> -	ret = V4L2Device::setFd(std::move(newFd));
> +	ret = V4L2Device::open(handle);
>  	if (ret < 0) {
> -		LOG(V4L2, Error) << "Failed to set file handle: "
> +		LOG(V4L2, Error) << "Failed to open file handle: "
>  				 << strerror(-ret);

V4L2Device::open() already complains out loud in case of failure. I
would drop the error message here.

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

>  		return ret;
>  	}
> --
> 2.37.3
>