[libcamera-devel] [PATCH v1 4/5] v4l2: v4l2_compat_manager: Store V4L2CameraFile in mmaps_

[paul.elder at ideasonboard.com]

Hi Laurent,

On Tue, Dec 28, 2021 at 11:59:50PM +0200, Laurent Pinchart wrote:
> The mmaps_ map stores a pointer to the V4L2CameraProxy to avoid
> increasing the reference count on the V4L2CameraFile shared pointer
> needlessly. While this provides a small optimization, it prevents
> accessing the V4L2CameraFile from the munmap() function which doesn't
> take an fd as argument.
> 
> To prepare for improved debugging that will require access to
> V4L2CameraFile in munmap(), store the V4L2CameraFile pointer in mmaps_.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

> ---
>  src/v4l2/v4l2_compat_manager.cpp | 10 +++-------
>  src/v4l2/v4l2_compat_manager.h   |  2 +-
>  2 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp
> index 585046e97e4b..ded568beb60e 100644
> --- a/src/v4l2/v4l2_compat_manager.cpp
> +++ b/src/v4l2/v4l2_compat_manager.cpp
> @@ -198,11 +198,7 @@ void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags,
>  	if (map == MAP_FAILED)
>  		return map;
>  
> -	/*
> -	 * Map to V4L2CameraProxy directly to prevent adding more references
> -	 * to V4L2CameraFile.
> -	 */
> -	mmaps_[map] = file->proxy();
> +	mmaps_[map] = file;
>  	return map;
>  }
>  
> @@ -212,9 +208,9 @@ int V4L2CompatManager::munmap(void *addr, size_t length)
>  	if (device == mmaps_.end())
>  		return fops_.munmap(addr, length);
>  
> -	V4L2CameraProxy *proxy = device->second;
> +	V4L2CameraFile *file = device->second.get();
>  
> -	int ret = proxy->munmap(addr, length);
> +	int ret = file->proxy()->munmap(addr, length);
>  	if (ret < 0)
>  		return ret;
>  
> diff --git a/src/v4l2/v4l2_compat_manager.h b/src/v4l2/v4l2_compat_manager.h
> index f52069f7b62d..64af9a8c008f 100644
> --- a/src/v4l2/v4l2_compat_manager.h
> +++ b/src/v4l2/v4l2_compat_manager.h
> @@ -65,5 +65,5 @@ private:
>  
>  	std::vector<std::unique_ptr<V4L2CameraProxy>> proxies_;
>  	std::map<int, std::shared_ptr<V4L2CameraFile>> files_;
> -	std::map<void *, V4L2CameraProxy *> mmaps_;
> +	std::map<void *, std::shared_ptr<V4L2CameraFile>> mmaps_;
>  };
> -- 
> Regards,
> 
> Laurent Pinchart
>