[PATCH v3 8/8] libcamera: base: log: Avoid manual `LogCategory` deletion

[Laurent Pinchart]

Hi Barnabás,

Thank you for the patch.

On Mon, Feb 17, 2025 at 06:55:10PM +0000, Barnabás Pőcze wrote:
> Wrap the `LogCategory` pointers in `std::unique_ptr` to avoid
> the need for manual deletion in the destructor.
> 
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> ---
>  src/libcamera/base/log.cpp | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index fd6c11716..e05ffc1c4 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -322,7 +322,7 @@ private:
>  	static bool destroyed_;
>  
>  	Mutex mutex_;
> -	std::vector<LogCategory *> categories_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
> +	std::vector<std::unique_ptr<LogCategory>> categories_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
>  	std::list<std::pair<std::string, LogSeverity>> levels_;
>  
>  	std::atomic<std::shared_ptr<LogOutput>> output_;
> @@ -439,9 +439,6 @@ void logSetLevel(const char *category, const char *level)
>  Logger::~Logger()
>  {
>  	destroyed_ = true;
> -
> -	for (LogCategory *category : categories_)
> -		delete category;
>  }
>  
>  /**
> @@ -574,7 +571,7 @@ void Logger::logSetLevel(const char *category, const char *level)
>  
>  	MutexLocker locker(mutex_);
>  
> -	for (LogCategory *c : categories_) {
> +	for (const auto &c : categories_) {
>  		if (c->name() == category) {
>  			c->setSeverity(severity);
>  			break;
> @@ -718,12 +715,12 @@ LogCategory *Logger::findOrCreateCategory(std::string_view name)
>  {
>  	MutexLocker locker(mutex_);
>  
> -	for (LogCategory *category : categories_) {
> +	for (const auto &category : categories_) {
>  		if (category->name() == name)
> -			return category;
> +			return category.get();
>  	}
>  
> -	LogCategory *category = categories_.emplace_back(new LogCategory(name));
> +	LogCategory *category = categories_.emplace_back(std::unique_ptr<LogCategory>(new LogCategory(name))).get();

I was going to write

	LogCategory *category = categories_.emplace_back(std::make_unique<LogCategory>(name)).get();

but the constructor is private. If we want to shorten the line,

	std::unique_ptr<LogCategory> cat{ new LogCategory(name) };
	LogCategory *category = categories_.emplace_back(std::move(cat)).get();

Up to you.

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

>  	const char *name_cstr = category->name().c_str();
>  
>  	for (const auto &[pattern, severity] : levels_) {

-- 
Regards,

Laurent Pinchart