[v1,1/3] libcamera: base: log: Take `LogCategory` by reference

22910 diff mbox series

When no log category is specified, `nullptr` is passed,
and then the `_log()` function implementations replace that
with `LogCategory::defaultCategory()`. But since the call site
always knows the log category, this condition can be removed
and the `_LOG1()` macro can use `LogCategory::defaultCategory()`.

So remove the condition from the `_log()` implementations and
use references to refer to log categories.
---
 include/libcamera/base/log.h | 8 ++++----
 src/libcamera/base/log.cpp   | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

Hi Barnabás,

Thank you for the patch.

On Mon, Mar 03, 2025 at 04:48:42PM +0100, Barnabás Pőcze wrote:
> When no log category is specified, `nullptr` is passed,
> and then the `_log()` function implementations replace that
> with `LogCategory::defaultCategory()`. But since the call site
> always knows the log category, this condition can be removed
> and the `_LOG1()` macro can use `LogCategory::defaultCategory()`.
> 
> So remove the condition from the `_log()` implementations and
> use references to refer to log categories.
> ---
>  include/libcamera/base/log.h | 8 ++++----
>  src/libcamera/base/log.cpp   | 8 ++++----
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h
> index 8af74b59d..958cb488d 100644
> --- a/include/libcamera/base/log.h
> +++ b/include/libcamera/base/log.h
> @@ -96,12 +96,12 @@ public:
>  protected:
>  	virtual std::string logPrefix() const = 0;
>  
> -	LogMessage _log(const LogCategory *category, LogSeverity severity,
> +	LogMessage _log(const LogCategory &category, LogSeverity severity,
>  			const char *fileName = __builtin_FILE(),
>  			unsigned int line = __builtin_LINE()) const;
>  };
>  
> -LogMessage _log(const LogCategory *category, LogSeverity severity,
> +LogMessage _log(const LogCategory &category, LogSeverity severity,
>  		const char *fileName = __builtin_FILE(),
>  		unsigned int line = __builtin_LINE());
>  
> @@ -109,9 +109,9 @@ LogMessage _log(const LogCategory *category, LogSeverity severity,
>  #define _LOG_CATEGORY(name) logCategory##name
>  
>  #define _LOG1(severity) \
> -	_log(nullptr, Log##severity).stream()
> +	_log(LogCategory::defaultCategory(), Log##severity).stream()
>  #define _LOG2(category, severity) \
> -	_log(&_LOG_CATEGORY(category)(), Log##severity).stream()
> +	_log(_LOG_CATEGORY(category)(), Log##severity).stream()
>  
>  /*
>   * Expand the LOG() macro to _LOG1() or _LOG2() based on the number of
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 6a040b592..ba57ad8f1 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -952,11 +952,11 @@ Loggable::~Loggable()
>   *
>   * \return A log message
>   */
> -LogMessage Loggable::_log(const LogCategory *category, LogSeverity severity,
> +LogMessage Loggable::_log(const LogCategory &category, LogSeverity severity,
>  			  const char *fileName, unsigned int line) const
>  {
>  	return LogMessage(fileName, line,
> -			  category ? *category : LogCategory::defaultCategory(),
> +			  category,
>  			  severity, logPrefix());

This now holds on a single line.

	return LogMessage(fileName, line, category, severity, logPrefix());

>  }
>  
> @@ -972,11 +972,11 @@ LogMessage Loggable::_log(const LogCategory *category, LogSeverity severity,
>   *
>   * \return A log message
>   */
> -LogMessage _log(const LogCategory *category, LogSeverity severity,
> +LogMessage _log(const LogCategory &category, LogSeverity severity,
>  		const char *fileName, unsigned int line)
>  {
>  	return LogMessage(fileName, line,
> -			  category ? *category : LogCategory::defaultCategory(),
> +			  category,
>  			  severity);

Same here.

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

>  }
>