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

[Sebastian Fricke]

Hey Laurent,

Thank you for the patch.

On 14.04.2021 00: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
>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.

Nice this patch is able to remove a lot of lines from the code.
I have tested this patch with `LIBCAMERA_LOG_LEVELS=0` (default category)
and with `LIBCAMERA_LOG_LEVELS=Camera:0` (with specified category).
And compared the output with the output before applying the patch.

In both cases the result looks exactly the same and there are no
problems.

>
>Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Tested-by: Sebastian Fricke <sebastian.fricke at posteo.net>

>---
> 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,
>
>Laurent Pinchart
>
>_______________________________________________
>libcamera-devel mailing list
>libcamera-devel at lists.libcamera.org
>https://lists.libcamera.org/listinfo/libcamera-devel