[libcamera-devel] [PATCH 1/4] v4l2: compat_manager: Move file operations to new struct FileOperations

[Kieran Bingham]

Hi Laurent,

On 06/01/2020 16:14, Laurent Pinchart wrote:
> Create a new FileOperations structure to hold all the dynamically-loaded
> C library file operations. The file operations are now exposed publicly,
> to prepare for usage of mmap in the V4L2CameraProxy.
> 
> A new template helper function is added to retrieve a symbol with
> dlsym() with proper casting to simplify the V4L2CompatManager
> constructor.

I like this,

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/v4l2/v4l2_compat_manager.cpp | 34 ++++++++++++++++++++------------
>  src/v4l2/v4l2_compat_manager.h   | 34 +++++++++++++++++++-------------
>  2 files changed, 41 insertions(+), 27 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp
> index a3d693745dd6..fe453445a9fb 100644
> --- a/src/v4l2/v4l2_compat_manager.cpp
> +++ b/src/v4l2/v4l2_compat_manager.cpp
> @@ -28,15 +28,23 @@ using namespace libcamera;
>  
>  LOG_DEFINE_CATEGORY(V4L2Compat)
>  
> +namespace {
> +template<typename T>
> +void get_symbol(T &func, const char *name)
> +{
> +	func = reinterpret_cast<T>(dlsym(RTLD_NEXT, name));
> +}
> +} /* namespace */
> +
>  V4L2CompatManager::V4L2CompatManager()
>  	: cm_(nullptr), initialized_(false)
>  {
> -	openat_func_ = (openat_func_t)dlsym(RTLD_NEXT, "openat");
> -	dup_func_    = (dup_func_t   )dlsym(RTLD_NEXT, "dup");
> -	close_func_  = (close_func_t )dlsym(RTLD_NEXT, "close");
> -	ioctl_func_  = (ioctl_func_t )dlsym(RTLD_NEXT, "ioctl");
> -	mmap_func_   = (mmap_func_t  )dlsym(RTLD_NEXT, "mmap");
> -	munmap_func_ = (munmap_func_t)dlsym(RTLD_NEXT, "munmap");
> +	get_symbol(fops_.openat, "openat");
> +	get_symbol(fops_.dup, "dup");
> +	get_symbol(fops_.close, "close");
> +	get_symbol(fops_.ioctl, "ioctl");
> +	get_symbol(fops_.mmap, "mmap");
> +	get_symbol(fops_.munmap, "munmap");
>  }
>  
>  V4L2CompatManager::~V4L2CompatManager()
> @@ -141,7 +149,7 @@ int V4L2CompatManager::getCameraIndex(int fd)
>  
>  int V4L2CompatManager::openat(int dirfd, const char *path, int oflag, mode_t mode)
>  {
> -	int fd = openat_func_(dirfd, path, oflag, mode);
> +	int fd = fops_.openat(dirfd, path, oflag, mode);
>  	if (fd < 0)
>  		return fd;
>  
> @@ -160,7 +168,7 @@ int V4L2CompatManager::openat(int dirfd, const char *path, int oflag, mode_t mod
>  		return fd;
>  	}
>  
> -	close_func_(fd);
> +	fops_.close(fd);
>  
>  	unsigned int camera_index = static_cast<unsigned int>(ret);
>  
> @@ -182,7 +190,7 @@ int V4L2CompatManager::openat(int dirfd, const char *path, int oflag, mode_t mod
>  
>  int V4L2CompatManager::dup(int oldfd)
>  {
> -	int newfd = dup_func_(oldfd);
> +	int newfd = fops_.dup(oldfd);
>  	if (newfd < 0)
>  		return newfd;
>  
> @@ -205,7 +213,7 @@ int V4L2CompatManager::close(int fd)
>  		return 0;
>  	}
>  
> -	return close_func_(fd);
> +	return fops_.close(fd);
>  }
>  
>  void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags,
> @@ -213,7 +221,7 @@ void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags,
>  {
>  	V4L2CameraProxy *proxy = getProxy(fd);
>  	if (!proxy)
> -		return mmap_func_(addr, length, prot, flags, fd, offset);
> +		return fops_.mmap(addr, length, prot, flags, fd, offset);
>  
>  	void *map = proxy->mmap(length, prot, flags, offset);
>  	if (map == MAP_FAILED)
> @@ -227,7 +235,7 @@ int V4L2CompatManager::munmap(void *addr, size_t length)
>  {
>  	auto device = mmaps_.find(addr);
>  	if (device == mmaps_.end())
> -		return munmap_func_(addr, length);
> +		return fops_.munmap(addr, length);
>  
>  	V4L2CameraProxy *proxy = device->second;
>  
> @@ -244,7 +252,7 @@ int V4L2CompatManager::ioctl(int fd, unsigned long request, void *arg)
>  {
>  	V4L2CameraProxy *proxy = getProxy(fd);
>  	if (!proxy)
> -		return ioctl_func_(fd, request, arg);
> +		return fops_.ioctl(fd, request, arg);
>  
>  	return proxy->ioctl(request, arg);
>  }
> diff --git a/src/v4l2/v4l2_compat_manager.h b/src/v4l2/v4l2_compat_manager.h
> index dd9d321078e4..d51b5953d930 100644
> --- a/src/v4l2/v4l2_compat_manager.h
> +++ b/src/v4l2/v4l2_compat_manager.h
> @@ -26,11 +26,30 @@ using namespace libcamera;
>  class V4L2CompatManager : public Thread
>  {
>  public:
> +	struct FileOperations {
> +		using openat_func_t = int (*)(int dirfd, const char *path,
> +					      int oflag, ...);
> +		using dup_func_t = int (*)(int oldfd);
> +		using close_func_t = int (*)(int fd);
> +		using ioctl_func_t = int (*)(int fd, unsigned long request, ...);
> +		using mmap_func_t = void *(*)(void *addr, size_t length, int prot,
> +					      int flags, int fd, off_t offset);
> +		using munmap_func_t = int (*)(void *addr, size_t length);
> +
> +		openat_func_t openat;
> +		dup_func_t dup;
> +		close_func_t close;
> +		ioctl_func_t ioctl;
> +		mmap_func_t mmap;
> +		munmap_func_t munmap;
> +	};
> +
>  	static V4L2CompatManager *instance();
>  
>  	int init();
>  
>  	V4L2CameraProxy *getProxy(int fd);
> +	const FileOperations &fops() const { return fops_; }
>  
>  	int openat(int dirfd, const char *path, int oflag, mode_t mode);
>  
> @@ -48,20 +67,7 @@ private:
>  	void run() override;
>  	int getCameraIndex(int fd);
>  
> -	typedef int (*openat_func_t)(int dirfd, const char *path, int oflag, ...);
> -	typedef int (*dup_func_t)(int oldfd);
> -	typedef int (*close_func_t)(int fd);
> -	typedef int (*ioctl_func_t)(int fd, unsigned long request, ...);
> -	typedef void *(*mmap_func_t)(void *addr, size_t length, int prot,
> -				     int flags, int fd, off_t offset);
> -	typedef int (*munmap_func_t)(void *addr, size_t length);
> -
> -	openat_func_t openat_func_;
> -	dup_func_t    dup_func_;
> -	close_func_t  close_func_;
> -	ioctl_func_t  ioctl_func_;
> -	mmap_func_t   mmap_func_;
> -	munmap_func_t munmap_func_;
> +	FileOperations fops_;
>  
>  	CameraManager *cm_;
>  
> 

-- 
Regards
--
Kieran