[RFC PATCH v2 9/9] libcamera: base: log: Store categories in list

[Jacopo Mondi]

Hi Barnabás

On Mon, Feb 03, 2025 at 05:59:39PM +0000, Barnabás Pőcze wrote:
> Store the `LogCategory` objects in an `std::list`. This eliminates
> the need for manually deleting them in the destructor while guaranteeing
> address stability.
>
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> ---
>  src/libcamera/base/log.cpp | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 69aa16c4c..003e2cf3a 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -407,7 +407,7 @@ private:
>  	static bool destroyed_;
>
>  	Mutex mutex_;
> -	std::vector<LogCategory *> categories_;
> +	std::list<LogCategory> categories_;

looks like the file didn't include <vector> !

>  	std::list<std::pair<std::string, LogSeverity>> levels_;
>
>  	std::shared_ptr<LogOutput> output_;
> @@ -524,9 +524,6 @@ void logSetLevel(const char *category, const char *level)
>  Logger::~Logger()
>  {
>  	destroyed_ = true;
> -
> -	for (LogCategory *category : categories_)
> -		delete category;
>  }
>
>  /**
> @@ -659,9 +656,9 @@ void Logger::logSetLevel(const char *category, const char *level)
>
>  	MutexLocker locker(mutex_);
>
> -	for (LogCategory *c : categories_) {
> -		if (c->name() == category) {
> -			c->setSeverity(severity);
> +	for (LogCategory &c : categories_) {
> +		if (c.name() == category) {
> +			c.setSeverity(severity);
>  			break;
>  		}
>  	}
> @@ -717,12 +714,12 @@ LogCategory *Logger::findOrCreateCategory(std::string_view name)
>  {
>  	MutexLocker locker(mutex_);
>
> -	for (LogCategory *category : categories_) {
> -		if (category->name() == name)
> -			return category;
> +	for (LogCategory &category : categories_) {
> +		if (category.name() == name)
> +			return &category;
>  	}
>
> -	LogCategory *category = categories_.emplace_back(new LogCategory(name));
> +	LogCategory &category = categories_.emplace_back(name);
>
>  	for (const std::pair<std::string, LogSeverity> &level : levels_) {
>  		bool match = true;
> @@ -739,12 +736,12 @@ LogCategory *Logger::findOrCreateCategory(std::string_view name)
>  		}
>
>  		if (match) {
> -			category->setSeverity(level.second);
> +			category.setSeverity(level.second);
>  			break;
>  		}
>  	}
>
> -	return category;
> +	return &category;

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

>  }
>
>  /**
> --
> 2.48.1
>
>