[v1] libcamera: base: thread: Use `std::unique_ptr` instead of raw pointer

24104 diff mbox series

An `std::unique_ptr` is safer and expresses the intent better, so use that.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
Or the same can be achieved by porting `Thread` to inherit `Extensible`.
---
 include/libcamera/base/thread.h |  2 +-
 src/libcamera/base/thread.cpp   | 18 ++++++++----------
 2 files changed, 9 insertions(+), 11 deletions(-)

--
2.50.1

On Thu, Aug 14, 2025 at 04:43:13PM +0200, Barnabás Pőcze wrote:
> An `std::unique_ptr` is safer and expresses the intent better, so use that.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> Or the same can be achieved by porting `Thread` to inherit `Extensible`.

Or we could possibly merge the Thread and ThreadData classes. I haven't
checked if there's a need to keep them separate.

> ---
>  include/libcamera/base/thread.h |  2 +-
>  src/libcamera/base/thread.cpp   | 18 ++++++++----------
>  2 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h
> index b9284c2c0..719d7eedb 100644
> --- a/include/libcamera/base/thread.h
> +++ b/include/libcamera/base/thread.h
> @@ -75,7 +75,7 @@ private:
>  			ThreadData *targetData);
> 
>  	std::thread thread_;
> -	ThreadData *data_;
> +	std::unique_ptr<ThreadData> data_;
>  };
> 
>  } /* namespace libcamera */
> diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp
> index d8fe0d697..92a7a8f7d 100644
> --- a/src/libcamera/base/thread.cpp
> +++ b/src/libcamera/base/thread.cpp
> @@ -105,8 +105,8 @@ public:
>  class ThreadData
>  {
>  public:
> -	ThreadData()
> -		: thread_(nullptr), running_(false), dispatcher_(nullptr)
> +	ThreadData(Thread *thread)
> +		: thread_(thread), running_(false), dispatcher_(nullptr)
>  	{
>  	}
> 
> @@ -167,7 +167,7 @@ ThreadData *ThreadData::current()
>  	 * The main thread doesn't receive thread-local data when it is
>  	 * started, set it here.
>  	 */
> -	ThreadData *data = mainThread.data_;
> +	ThreadData *data = mainThread.data_.get();
>  	data->tid_ = syscall(SYS_gettid);
>  	currentThreadData = data;
>  	return data;
> @@ -231,15 +231,13 @@ ThreadData *ThreadData::current()
>   * \brief Create a thread
>   */
>  Thread::Thread()
> +	: data_(std::make_unique<ThreadData>(this))
>  {
> -	data_ = new ThreadData;
> -	data_->thread_ = this;
>  }
> 
>  Thread::~Thread()
>  {
>  	delete data_->dispatcher_.load(std::memory_order_relaxed);
> -	delete data_;
>  }
> 
>  /**
> @@ -284,7 +282,7 @@ void Thread::startThread()
>  	thread_local ThreadCleaner cleaner(this, &Thread::finishThread);
> 
>  	data_->tid_ = syscall(SYS_gettid);
> -	currentThreadData = data_;
> +	currentThreadData = data_.get();
> 
>  	run();
>  }
> @@ -621,7 +619,7 @@ void Thread::removeMessages(Object *receiver)
>   */
>  void Thread::dispatchMessages(Message::Type type, Object *receiver)
>  {
> -	ASSERT(data_ == ThreadData::current());
> +	ASSERT(data_.get() == ThreadData::current());
> 
>  	++data_->messages_.recursion_;
> 
> @@ -677,8 +675,8 @@ void Thread::dispatchMessages(Message::Type type, Object *receiver)
>   */
>  void Thread::moveObject(Object *object)
>  {
> -	ThreadData *currentData = object->thread_->data_;
> -	ThreadData *targetData = data_;
> +	ThreadData *currentData = object->thread_->data_.get();
> +	ThreadData *targetData = data_.get();
> 
>  	MutexLocker lockerFrom(currentData->messages_.mutex_, std::defer_lock);
>  	MutexLocker lockerTo(targetData->messages_.mutex_, std::defer_lock);