[libcamera-devel] [PATCH 1/2] libcamera: log: De-duplicate _log() functions and LogMessage constructor

[Kieran Bingham]

Hi Laurent,

On 13/04/2021 22:51, Laurent Pinchart wrote:
> The _log() functions, as well as the LogMessage constructor, exist in
> two versions, one that takes a log category, and one that doesn't. The
> latter uses the default log category. This can be simplied by passing a

s/simplied/simplified/

> LogCategory pointer to _log(), which can then be null for the default
> category, and moving the retrieval of the default log category from the
> LogMessage constructor to the _log() function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Looks good to me.

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  include/libcamera/internal/log.h | 13 ++-----
>  src/libcamera/log.cpp            | 66 ++++----------------------------
>  2 files changed, 12 insertions(+), 67 deletions(-)
> 
> diff --git a/include/libcamera/internal/log.h b/include/libcamera/internal/log.h
> index e76ae913c857..0fdacc4733fe 100644
> --- a/include/libcamera/internal/log.h
> +++ b/include/libcamera/internal/log.h
> @@ -55,8 +55,6 @@ const LogCategory &_LOG_CATEGORY(name)()				\
>  class LogMessage
>  {
>  public:
> -	LogMessage(const char *fileName, unsigned int line,
> -		   LogSeverity severity);
>  	LogMessage(const char *fileName, unsigned int line,
>  		   const LogCategory &category, LogSeverity severity);
>  
> @@ -92,23 +90,20 @@ protected:
>  	virtual std::string logPrefix() const = 0;
>  
>  	LogMessage _log(const char *file, unsigned int line,
> -			LogSeverity severity) const;
> -	LogMessage _log(const char *file, unsigned int line,
> -			const LogCategory &category,
> +			const LogCategory *category,
>  			LogSeverity severity) const;
>  };
>  
> -LogMessage _log(const char *file, unsigned int line, LogSeverity severity);
>  LogMessage _log(const char *file, unsigned int line,
> -		const LogCategory &category, LogSeverity severity);
> +		const LogCategory *category, LogSeverity severity);
>  
>  #ifndef __DOXYGEN__
>  #define _LOG_CATEGORY(name) logCategory##name
>  
>  #define _LOG1(severity) \
> -	_log(__FILE__, __LINE__, Log##severity).stream()
> +	_log(__FILE__, __LINE__, nullptr, Log##severity).stream()
>  #define _LOG2(category, severity) \
> -	_log(__FILE__, __LINE__, _LOG_CATEGORY(category)(), Log##severity).stream()
> +	_log(__FILE__, __LINE__, &_LOG_CATEGORY(category)(), Log##severity).stream()
>  
>  /*
>   * Expand the LOG() macro to _LOG1() or _LOG2() based on the number of
> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp
> index 45c7c2d24652..9f86e645ac58 100644
> --- a/src/libcamera/log.cpp
> +++ b/src/libcamera/log.cpp
> @@ -763,24 +763,6 @@ const LogCategory &LogCategory::defaultCategory()
>   * directly. Use the LOG() macro instead access the log infrastructure.
>   */
>  
> -/**
> - * \brief Construct a log message for the default category
> - * \param[in] fileName The file name where the message is logged from
> - * \param[in] line The line number where the message is logged from
> - * \param[in] severity The log message severity, controlling how the message
> - * will be displayed
> - *
> - * Create a log message pertaining to line \a line of file \a fileName. The
> - * \a severity argument sets the message severity to control whether it will be
> - * output or dropped.
> - */
> -LogMessage::LogMessage(const char *fileName, unsigned int line,
> -		       LogSeverity severity)
> -	: category_(LogCategory::defaultCategory()), severity_(severity)
> -{
> -	init(fileName, line);
> -}
> -
>  /**
>   * \brief Construct a log message for a given category
>   * \param[in] fileName The file name where the message is logged from
> @@ -913,26 +895,6 @@ Loggable::~Loggable()
>   * \return A string to be prefixed to the log message
>   */
>  
> -/**
> - * \brief Create a temporary LogMessage object to log a message
> - * \param[in] fileName The file name where the message is logged from
> - * \param[in] line The line number where the message is logged from
> - * \param[in] severity The log message severity
> - *
> - * This method is used as a backeng by the LOG() macro to create a log message
> - * for locations inheriting from the Loggable class.
> - *
> - * \return A log message
> - */
> -LogMessage Loggable::_log(const char *fileName, unsigned int line,
> -			  LogSeverity severity) const
> -{
> -	LogMessage msg(fileName, line, severity);
> -
> -	msg.stream() << logPrefix() << ": ";
> -	return msg;
> -}
> -
>  /**
>   * \brief Create a temporary LogMessage object to log a message
>   * \param[in] fileName The file name where the message is logged from
> @@ -946,31 +908,17 @@ LogMessage Loggable::_log(const char *fileName, unsigned int line,
>   * \return A log message
>   */
>  LogMessage Loggable::_log(const char *fileName, unsigned int line,
> -			  const LogCategory &category,
> +			  const LogCategory *category,
>  			  LogSeverity severity) const
>  {
> -	LogMessage msg(fileName, line, category, severity);
> +	LogMessage msg(fileName, line,
> +		       category ? *category : LogCategory::defaultCategory(),
> +		       severity);
>  
>  	msg.stream() << logPrefix() << ": ";
>  	return msg;
>  }
>  
> -/**
> - * \brief Create a temporary LogMessage object to log a message
> - * \param[in] fileName The file name where the message is logged from
> - * \param[in] line The line number where the message is logged from
> - * \param[in] severity The log message severity
> - *
> - * This function is used as a backeng by the LOG() macro to create a log
> - * message for locations not inheriting from the Loggable class.
> - *
> - * \return A log message
> - */
> -LogMessage _log(const char *fileName, unsigned int line, LogSeverity severity)
> -{
> -	return LogMessage(fileName, line, severity);
> -}
> -
>  /**
>   * \brief Create a temporary LogMessage object to log a message
>   * \param[in] fileName The file name where the message is logged from
> @@ -984,9 +932,11 @@ LogMessage _log(const char *fileName, unsigned int line, LogSeverity severity)
>   * \return A log message
>   */
>  LogMessage _log(const char *fileName, unsigned int line,
> -		const LogCategory &category, LogSeverity severity)
> +		const LogCategory *category, LogSeverity severity)
>  {
> -	return LogMessage(fileName, line, category, severity);
> +	return LogMessage(fileName, line,
> +			  category ? *category : LogCategory::defaultCategory(),
> +			  severity);
>  }
>  
>  /**
> 

-- 
Regards
--
Kieran