[PATCH v5 05/18] libcamera: shared_mem_object: reorganize the code and document the SharedMemObject class

[Barnabás Pőcze]

Hi


2024. március 11., hétfő 15:15 keltezéssel, Hans de Goede <hdegoede at redhat.com> írta:

> From: Andrei Konovalov <andrey.konovalov.ynk at gmail.com>
> 
> Split the parts which doesn't otherwise depend on the type T or
> arguments Args out of the SharedMemObject class into a new
> SharedMem class.
> 
> Doxygen documentation by Dennis Bonke and Andrei Konovalov.
> 
> Reviewed-by: Pavel Machek <pavel at ucw.cz>
> Co-developed-by: Dennis Bonke <admin at dennisbonke.com>
> Signed-off-by: Dennis Bonke <admin at dennisbonke.com>
> Signed-off-by: Andrei Konovalov <andrey.konovalov.ynk at gmail.com>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  .../libcamera/internal/shared_mem_object.h    | 101 ++++++----
>  src/libcamera/meson.build                     |   1 +
>  src/libcamera/shared_mem_object.cpp           | 190 ++++++++++++++++++
>  3 files changed, 253 insertions(+), 39 deletions(-)
>  create mode 100644 src/libcamera/shared_mem_object.cpp
> 
> [...]
>  template<class T>
> -class SharedMemObject
> +class SharedMemObject : public SharedMem
>  {
>  public:
>  	static constexpr std::size_t SIZE = sizeof(T);
> 
>  	SharedMemObject()
> -		: obj_(nullptr)
> +		: SharedMem(), obj_(nullptr)
>  	{
>  	}
> 
>  	template<class... Args>
>  	SharedMemObject(const std::string &name, Args &&...args)
> -		: name_(name), obj_(nullptr)
> +		: SharedMem(name, SIZE), obj_(nullptr)
>  	{
> -		void *mem;
> -		int ret;
> -
> -		ret = memfd_create(name_.c_str(), MFD_CLOEXEC);
> -		if (ret < 0)
> +		if (mem_ == nullptr)
>  			return;
> 
> -		fd_ = SharedFD(std::move(ret));
> -		if (!fd_.isValid())
> -			return;
> -
> -		ret = ftruncate(fd_.get(), SIZE);
> -		if (ret < 0)
> -			return;
> -
> -		mem = mmap(nullptr, SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,
> -			   fd_.get(), 0);
> -		if (mem == MAP_FAILED)
> -			return;
> -
> -		obj_ = new (mem) T(std::forward<Args>(args)...);
> +		obj_ = new (mem_) T(std::forward<Args>(args)...);

If I see it correctly, every `SharedMemObject` stores the same pointer twice
(`obj_` and `mem_` in the base class). I think this can be avoided.


>  	}
> 
>  	SharedMemObject(SharedMemObject<T> &&rhs)
> +		: SharedMem(std::move(rhs))
>  	{
> -		this->name_ = std::move(rhs.name_);
> -		this->fd_ = std::move(rhs.fd_);
>  		this->obj_ = rhs.obj_;
>  		rhs.obj_ = nullptr;
>  	}
> 
>  	~SharedMemObject()
>  	{
> -		if (obj_) {
> +		if (obj_)
>  			obj_->~T();
> -			munmap(obj_, SIZE);
> -		}
>  	}
> 
>  	/* Make SharedMemObject non-copyable for now. */
> @@ -78,8 +109,7 @@ public:
> 
>  	SharedMemObject<T> &operator=(SharedMemObject<T> &&rhs)
>  	{
> -		this->name_ = std::move(rhs.name_);
> -		this->fd_ = std::move(rhs.fd_);
> +		SharedMem::operator=(std::move(rhs));
>  		this->obj_ = rhs.obj_;
>  		rhs.obj_ = nullptr;
>  		return *this;
> @@ -105,19 +135,12 @@ public:
>  		return *obj_;
>  	}
> 
> -	const SharedFD &fd() const
> -	{
> -		return fd_;
> -	}
> -
>  	explicit operator bool() const
>  	{
>  		return !!obj_;
>  	}
> 
>  private:
> -	std::string name_;
> -	SharedFD fd_;
>  	T *obj_;
>  };
> 
> [...]


Regards,
Barnabás Pőcze