Message ID | 20250130195811.1230581-8-pobrn@protonmail.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
Hi Barnabás On Thu, Jan 30, 2025 at 07:58:52PM +0000, Barnabás Pőcze wrote: > These two functions do not need to be exposed in any > way, nor do they need to be a member of the `Logger` class. > > So move them into an anonymous namespace. > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > --- > src/libcamera/base/log.cpp | 186 ++++++++++++++++++------------------- > 1 file changed, 93 insertions(+), 93 deletions(-) > > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp > index 0dfdb0e9b..19fc5cc67 100644 > --- a/src/libcamera/base/log.cpp > +++ b/src/libcamera/base/log.cpp > @@ -65,7 +65,9 @@ > > namespace libcamera { > > -static int log_severity_to_syslog(LogSeverity severity) > +namespace { > + > +int log_severity_to_syslog(LogSeverity severity) > { > switch (severity) { > case LogDebug: > @@ -83,7 +85,7 @@ static int log_severity_to_syslog(LogSeverity severity) > } > } > > -static const char *log_severity_name(LogSeverity severity) > +const char *log_severity_name(LogSeverity severity) > { > static const char *const names[] = { > "DEBUG", > @@ -99,6 +101,92 @@ static const char *log_severity_name(LogSeverity severity) > return "UNKWN"; > } > > +/** Maybe they don't need to be doxygened anymore ? Not a big deal Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > + * \brief Parse a log level string into a LogSeverity > + * \param[in] level The log level string > + * > + * Log levels can be specified as an integer value in the range from LogDebug to > + * LogFatal, or as a string corresponding to the severity name in uppercase. Any > + * other value is invalid. > + * > + * \return The log severity, or LogInvalid if the string is invalid > + */ > +LogSeverity parseLogLevel(std::string_view level) > +{ > + static const char *const names[] = { > + "DEBUG", > + "INFO", > + "WARN", > + "ERROR", > + "FATAL", > + }; > + > + unsigned int severity; > + > + if (std::isdigit(level[0])) { > + auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity); > + if (ec != std::errc() || *end != '\0' || severity > LogFatal) > + severity = LogInvalid; > + } else { > + severity = LogInvalid; > + for (unsigned int i = 0; i < std::size(names); ++i) { > + if (names[i] == level) { > + severity = i; > + break; > + } > + } > + } > + > + return static_cast<LogSeverity>(severity); > +} > + > +/** > + * \brief Parse the log levels from the environment > + * > + * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable > + * as a list of "category:level" pairs, separated by commas (','). Parse the > + * variable and store the levels to configure all log categories. > + */ > +void parseLogLevels(const char *line, std::list<std::pair<std::string, LogSeverity>> &levels) > +{ > + for (const char *pair = line; *line != '\0'; pair = line) { > + const char *comma = strchrnul(line, ','); > + size_t len = comma - pair; > + > + /* Skip over the comma. */ > + line = *comma == ',' ? comma + 1 : comma; > + > + /* Skip to the next pair if the pair is empty. */ > + if (!len) > + continue; > + > + std::string_view category; > + std::string_view level; > + > + const char *colon = static_cast<const char *>(memchr(pair, ':', len)); > + if (!colon) { > + /* 'x' is a shortcut for '*:x'. */ > + category = "*"; > + level = std::string_view(pair, len); > + } else { > + category = std::string_view(pair, colon - pair); > + level = std::string_view(colon + 1, comma - colon - 1); > + } > + > + /* Both the category and the level must be specified. */ > + if (category.empty() || level.empty()) > + continue; > + > + LogSeverity severity = parseLogLevel(level); > + if (severity == LogInvalid) > + continue; > + > + levels.emplace_back(category, severity); > + } > +} > + > +} /* namespace */ > + > /** > * \brief Log output > * > @@ -312,8 +400,6 @@ private: > Logger(); > > void parseLogFile(); > - void parseLogLevels(); > - static LogSeverity parseLogLevel(std::string_view level); > > friend LogCategory; > void registerCategory(LogCategory *category); > @@ -592,7 +678,9 @@ Logger::Logger() > logSetStream(&std::cerr, color); > > parseLogFile(); > - parseLogLevels(); > + > + if (const char *debug = utils::secure_getenv("LIBCAMERA_LOG_LEVELS")) > + parseLogLevels(debug, levels_); > } > > /** > @@ -618,94 +706,6 @@ void Logger::parseLogFile() > logSetFile(file, false); > } > > -/** > - * \brief Parse the log levels from the environment > - * > - * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable > - * as a list of "category:level" pairs, separated by commas (','). Parse the > - * variable and store the levels to configure all log categories. > - */ > -void Logger::parseLogLevels() > -{ > - const char *debug = utils::secure_getenv("LIBCAMERA_LOG_LEVELS"); > - if (!debug) > - return; > - > - for (const char *pair = debug; *debug != '\0'; pair = debug) { > - const char *comma = strchrnul(debug, ','); > - size_t len = comma - pair; > - > - /* Skip over the comma. */ > - debug = *comma == ',' ? comma + 1 : comma; > - > - /* Skip to the next pair if the pair is empty. */ > - if (!len) > - continue; > - > - std::string_view category; > - std::string_view level; > - > - const char *colon = static_cast<const char *>(memchr(pair, ':', len)); > - if (!colon) { > - /* 'x' is a shortcut for '*:x'. */ > - category = "*"; > - level = std::string_view(pair, len); > - } else { > - category = std::string_view(pair, colon - pair); > - level = std::string_view(colon + 1, comma - colon - 1); > - } > - > - /* Both the category and the level must be specified. */ > - if (category.empty() || level.empty()) > - continue; > - > - LogSeverity severity = parseLogLevel(level); > - if (severity == LogInvalid) > - continue; > - > - levels_.emplace_back(category, severity); > - } > -} > - > -/** > - * \brief Parse a log level string into a LogSeverity > - * \param[in] level The log level string > - * > - * Log levels can be specified as an integer value in the range from LogDebug to > - * LogFatal, or as a string corresponding to the severity name in uppercase. Any > - * other value is invalid. > - * > - * \return The log severity, or LogInvalid if the string is invalid > - */ > -LogSeverity Logger::parseLogLevel(std::string_view level) > -{ > - static const char *const names[] = { > - "DEBUG", > - "INFO", > - "WARN", > - "ERROR", > - "FATAL", > - }; > - > - unsigned int severity; > - > - if (std::isdigit(level[0])) { > - auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity); > - if (ec != std::errc() || *end != '\0' || severity > LogFatal) > - severity = LogInvalid; > - } else { > - severity = LogInvalid; > - for (unsigned int i = 0; i < std::size(names); ++i) { > - if (names[i] == level) { > - severity = i; > - break; > - } > - } > - } > - > - return static_cast<LogSeverity>(severity); > -} > - > /** > * \brief Register a log category with the logger > * \param[in] category The log category > -- > 2.48.1 > >
Hi Barnab@s, Thank you for the patch. On Thu, Jan 30, 2025 at 07:58:52PM +0000, Barnabás Pőcze wrote: > These two functions do not need to be exposed in any > way, nor do they need to be a member of the `Logger` class. For parseLogLevels() this is not actually true. The function accesses a member variable, which you now pass it as a function argument. What's the advantage of this patch ? > So move them into an anonymous namespace. > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > --- > src/libcamera/base/log.cpp | 186 ++++++++++++++++++------------------- > 1 file changed, 93 insertions(+), 93 deletions(-) > > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp > index 0dfdb0e9b..19fc5cc67 100644 > --- a/src/libcamera/base/log.cpp > +++ b/src/libcamera/base/log.cpp > @@ -65,7 +65,9 @@ > > namespace libcamera { > > -static int log_severity_to_syslog(LogSeverity severity) > +namespace { > + > +int log_severity_to_syslog(LogSeverity severity) > { > switch (severity) { > case LogDebug: > @@ -83,7 +85,7 @@ static int log_severity_to_syslog(LogSeverity severity) > } > } > > -static const char *log_severity_name(LogSeverity severity) > +const char *log_severity_name(LogSeverity severity) > { > static const char *const names[] = { > "DEBUG", > @@ -99,6 +101,92 @@ static const char *log_severity_name(LogSeverity severity) > return "UNKWN"; > } > > +/** > + * \brief Parse a log level string into a LogSeverity > + * \param[in] level The log level string > + * > + * Log levels can be specified as an integer value in the range from LogDebug to > + * LogFatal, or as a string corresponding to the severity name in uppercase. Any > + * other value is invalid. > + * > + * \return The log severity, or LogInvalid if the string is invalid > + */ > +LogSeverity parseLogLevel(std::string_view level) > +{ > + static const char *const names[] = { > + "DEBUG", > + "INFO", > + "WARN", > + "ERROR", > + "FATAL", > + }; > + > + unsigned int severity; > + > + if (std::isdigit(level[0])) { > + auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity); > + if (ec != std::errc() || *end != '\0' || severity > LogFatal) > + severity = LogInvalid; > + } else { > + severity = LogInvalid; > + for (unsigned int i = 0; i < std::size(names); ++i) { > + if (names[i] == level) { > + severity = i; > + break; > + } > + } > + } > + > + return static_cast<LogSeverity>(severity); > +} > + > +/** > + * \brief Parse the log levels from the environment Missing \param > + * > + * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable > + * as a list of "category:level" pairs, separated by commas (','). Parse the > + * variable and store the levels to configure all log categories. > + */ > +void parseLogLevels(const char *line, std::list<std::pair<std::string, LogSeverity>> &levels) > +{ > + for (const char *pair = line; *line != '\0'; pair = line) { > + const char *comma = strchrnul(line, ','); > + size_t len = comma - pair; > + > + /* Skip over the comma. */ > + line = *comma == ',' ? comma + 1 : comma; > + > + /* Skip to the next pair if the pair is empty. */ > + if (!len) > + continue; > + > + std::string_view category; > + std::string_view level; > + > + const char *colon = static_cast<const char *>(memchr(pair, ':', len)); > + if (!colon) { > + /* 'x' is a shortcut for '*:x'. */ > + category = "*"; > + level = std::string_view(pair, len); > + } else { > + category = std::string_view(pair, colon - pair); > + level = std::string_view(colon + 1, comma - colon - 1); > + } > + > + /* Both the category and the level must be specified. */ > + if (category.empty() || level.empty()) > + continue; > + > + LogSeverity severity = parseLogLevel(level); > + if (severity == LogInvalid) > + continue; > + > + levels.emplace_back(category, severity); > + } > +} > + > +} /* namespace */ > + > /** > * \brief Log output > * > @@ -312,8 +400,6 @@ private: > Logger(); > > void parseLogFile(); > - void parseLogLevels(); > - static LogSeverity parseLogLevel(std::string_view level); > > friend LogCategory; > void registerCategory(LogCategory *category); > @@ -592,7 +678,9 @@ Logger::Logger() > logSetStream(&std::cerr, color); > > parseLogFile(); > - parseLogLevels(); > + > + if (const char *debug = utils::secure_getenv("LIBCAMERA_LOG_LEVELS")) > + parseLogLevels(debug, levels_); > } > > /** > @@ -618,94 +706,6 @@ void Logger::parseLogFile() > logSetFile(file, false); > } > > -/** > - * \brief Parse the log levels from the environment > - * > - * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable > - * as a list of "category:level" pairs, separated by commas (','). Parse the > - * variable and store the levels to configure all log categories. > - */ > -void Logger::parseLogLevels() > -{ > - const char *debug = utils::secure_getenv("LIBCAMERA_LOG_LEVELS"); > - if (!debug) > - return; > - > - for (const char *pair = debug; *debug != '\0'; pair = debug) { > - const char *comma = strchrnul(debug, ','); > - size_t len = comma - pair; > - > - /* Skip over the comma. */ > - debug = *comma == ',' ? comma + 1 : comma; > - > - /* Skip to the next pair if the pair is empty. */ > - if (!len) > - continue; > - > - std::string_view category; > - std::string_view level; > - > - const char *colon = static_cast<const char *>(memchr(pair, ':', len)); > - if (!colon) { > - /* 'x' is a shortcut for '*:x'. */ > - category = "*"; > - level = std::string_view(pair, len); > - } else { > - category = std::string_view(pair, colon - pair); > - level = std::string_view(colon + 1, comma - colon - 1); > - } > - > - /* Both the category and the level must be specified. */ > - if (category.empty() || level.empty()) > - continue; > - > - LogSeverity severity = parseLogLevel(level); > - if (severity == LogInvalid) > - continue; > - > - levels_.emplace_back(category, severity); > - } > -} > - > -/** > - * \brief Parse a log level string into a LogSeverity > - * \param[in] level The log level string > - * > - * Log levels can be specified as an integer value in the range from LogDebug to > - * LogFatal, or as a string corresponding to the severity name in uppercase. Any > - * other value is invalid. > - * > - * \return The log severity, or LogInvalid if the string is invalid > - */ > -LogSeverity Logger::parseLogLevel(std::string_view level) > -{ > - static const char *const names[] = { > - "DEBUG", > - "INFO", > - "WARN", > - "ERROR", > - "FATAL", > - }; > - > - unsigned int severity; > - > - if (std::isdigit(level[0])) { > - auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity); > - if (ec != std::errc() || *end != '\0' || severity > LogFatal) > - severity = LogInvalid; > - } else { > - severity = LogInvalid; > - for (unsigned int i = 0; i < std::size(names); ++i) { > - if (names[i] == level) { > - severity = i; > - break; > - } > - } > - } > - > - return static_cast<LogSeverity>(severity); > -} > - > /** > * \brief Register a log category with the logger > * \param[in] category The log category
Hi 2025. február 6., csütörtök 17:48 keltezéssel, Laurent Pinchart <laurent.pinchart@ideasonboard.com> írta: > Hi Barnab@s, > > Thank you for the patch. > > On Thu, Jan 30, 2025 at 07:58:52PM +0000, Barnabás Pőcze wrote: > > These two functions do not need to be exposed in any > > way, nor do they need to be a member of the `Logger` class. > > For parseLogLevels() this is not actually true. The function accesses a > member variable, which you now pass it as a function argument. What's > the advantage of this patch ? Ahh, you're right. The motivation is to decouple the log level specification string source (the environment variable) and destination (`levels_` list) from the parsing itself. This was done mostly because I was considering potential solutions to https://bugs.libcamera.org/show_bug.cgi?id=243. Another thing was the introduction of the lock in the next patch, keeping the number of places where a mutex protected object is accessed seems like a good idea (since it is not enforced by any kind of wrapper type) (although since `parseLogLevels()` is only called from the constructor, I opted not to do any locking there in the next patch). > > > So move them into an anonymous namespace. > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > --- > > src/libcamera/base/log.cpp | 186 ++++++++++++++++++------------------- > > 1 file changed, 93 insertions(+), 93 deletions(-) > > > > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp > > index 0dfdb0e9b..19fc5cc67 100644 > > --- a/src/libcamera/base/log.cpp > > +++ b/src/libcamera/base/log.cpp > > @@ -65,7 +65,9 @@ > > > > namespace libcamera { > > > > -static int log_severity_to_syslog(LogSeverity severity) > > +namespace { > > + > > +int log_severity_to_syslog(LogSeverity severity) > > { > > switch (severity) { > > case LogDebug: > > @@ -83,7 +85,7 @@ static int log_severity_to_syslog(LogSeverity severity) > > } > > } > > > > -static const char *log_severity_name(LogSeverity severity) > > +const char *log_severity_name(LogSeverity severity) > > { > > static const char *const names[] = { > > "DEBUG", > > @@ -99,6 +101,92 @@ static const char *log_severity_name(LogSeverity severity) > > return "UNKWN"; > > } > > > > +/** > > + * \brief Parse a log level string into a LogSeverity > > + * \param[in] level The log level string > > + * > > + * Log levels can be specified as an integer value in the range from LogDebug to > > + * LogFatal, or as a string corresponding to the severity name in uppercase. Any > > + * other value is invalid. > > + * > > + * \return The log severity, or LogInvalid if the string is invalid > > + */ > > +LogSeverity parseLogLevel(std::string_view level) > > +{ > > + static const char *const names[] = { > > + "DEBUG", > > + "INFO", > > + "WARN", > > + "ERROR", > > + "FATAL", > > + }; > > + > > + unsigned int severity; > > + > > + if (std::isdigit(level[0])) { > > + auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity); > > + if (ec != std::errc() || *end != '\0' || severity > LogFatal) > > + severity = LogInvalid; > > + } else { > > + severity = LogInvalid; > > + for (unsigned int i = 0; i < std::size(names); ++i) { > > + if (names[i] == level) { > > + severity = i; > > + break; > > + } > > + } > > + } > > + > > + return static_cast<LogSeverity>(severity); > > +} > > + > > +/** > > + * \brief Parse the log levels from the environment > > Missing \param ACK > > > + * > > + * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable > > + * as a list of "category:level" pairs, separated by commas (','). Parse the > > + * variable and store the levels to configure all log categories. > > + */ > > +void parseLogLevels(const char *line, std::list<std::pair<std::string, LogSeverity>> &levels) > > +{ > > + for (const char *pair = line; *line != '\0'; pair = line) { > > + const char *comma = strchrnul(line, ','); > > + size_t len = comma - pair; > > + > > + /* Skip over the comma. */ > > + line = *comma == ',' ? comma + 1 : comma; > > + > > + /* Skip to the next pair if the pair is empty. */ > > + if (!len) > > + continue; > > + > > + std::string_view category; > > + std::string_view level; > > + > > + const char *colon = static_cast<const char *>(memchr(pair, ':', len)); > > + if (!colon) { > > + /* 'x' is a shortcut for '*:x'. */ > > + category = "*"; > > + level = std::string_view(pair, len); > > + } else { > > + category = std::string_view(pair, colon - pair); > > + level = std::string_view(colon + 1, comma - colon - 1); > > + } > > + > > + /* Both the category and the level must be specified. */ > > + if (category.empty() || level.empty()) > > + continue; > > + > > + LogSeverity severity = parseLogLevel(level); > > + if (severity == LogInvalid) > > + continue; > > + > > + levels.emplace_back(category, severity); > > + } > > +} > > + > > +} /* namespace */ > > + > > /** > > * \brief Log output > > * > > @@ -312,8 +400,6 @@ private: > > Logger(); > > > > void parseLogFile(); > > - void parseLogLevels(); > > - static LogSeverity parseLogLevel(std::string_view level); > > > > friend LogCategory; > > void registerCategory(LogCategory *category); > > @@ -592,7 +678,9 @@ Logger::Logger() > > logSetStream(&std::cerr, color); > > > > parseLogFile(); > > - parseLogLevels(); > > + > > + if (const char *debug = utils::secure_getenv("LIBCAMERA_LOG_LEVELS")) > > + parseLogLevels(debug, levels_); > > } > > > > /** > > @@ -618,94 +706,6 @@ void Logger::parseLogFile() > > logSetFile(file, false); > > } > > > > -/** > > - * \brief Parse the log levels from the environment > > - * > > - * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable > > - * as a list of "category:level" pairs, separated by commas (','). Parse the > > - * variable and store the levels to configure all log categories. > > - */ > > -void Logger::parseLogLevels() > > -{ > > - const char *debug = utils::secure_getenv("LIBCAMERA_LOG_LEVELS"); > > - if (!debug) > > - return; > > - > > - for (const char *pair = debug; *debug != '\0'; pair = debug) { > > - const char *comma = strchrnul(debug, ','); > > - size_t len = comma - pair; > > - > > - /* Skip over the comma. */ > > - debug = *comma == ',' ? comma + 1 : comma; > > - > > - /* Skip to the next pair if the pair is empty. */ > > - if (!len) > > - continue; > > - > > - std::string_view category; > > - std::string_view level; > > - > > - const char *colon = static_cast<const char *>(memchr(pair, ':', len)); > > - if (!colon) { > > - /* 'x' is a shortcut for '*:x'. */ > > - category = "*"; > > - level = std::string_view(pair, len); > > - } else { > > - category = std::string_view(pair, colon - pair); > > - level = std::string_view(colon + 1, comma - colon - 1); > > - } > > - > > - /* Both the category and the level must be specified. */ > > - if (category.empty() || level.empty()) > > - continue; > > - > > - LogSeverity severity = parseLogLevel(level); > > - if (severity == LogInvalid) > > - continue; > > - > > - levels_.emplace_back(category, severity); > > - } > > -} > > - > > -/** > > - * \brief Parse a log level string into a LogSeverity > > - * \param[in] level The log level string > > - * > > - * Log levels can be specified as an integer value in the range from LogDebug to > > - * LogFatal, or as a string corresponding to the severity name in uppercase. Any > > - * other value is invalid. > > - * > > - * \return The log severity, or LogInvalid if the string is invalid > > - */ > > -LogSeverity Logger::parseLogLevel(std::string_view level) > > -{ > > - static const char *const names[] = { > > - "DEBUG", > > - "INFO", > > - "WARN", > > - "ERROR", > > - "FATAL", > > - }; > > - > > - unsigned int severity; > > - > > - if (std::isdigit(level[0])) { > > - auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity); > > - if (ec != std::errc() || *end != '\0' || severity > LogFatal) > > - severity = LogInvalid; > > - } else { > > - severity = LogInvalid; > > - for (unsigned int i = 0; i < std::size(names); ++i) { > > - if (names[i] == level) { > > - severity = i; > > - break; > > - } > > - } > > - } > > - > > - return static_cast<LogSeverity>(severity); > > -} > > - > > /** > > * \brief Register a log category with the logger > > * \param[in] category The log category > > -- > Regards, > > Laurent Pinchart >
On Thu, Feb 06, 2025 at 05:18:55PM +0000, Barnabás Pőcze wrote: > 2025. február 6., csütörtök 17:48 keltezéssel, Laurent Pinchart írta: > > On Thu, Jan 30, 2025 at 07:58:52PM +0000, Barnabás Pőcze wrote: > > > These two functions do not need to be exposed in any > > > way, nor do they need to be a member of the `Logger` class. > > > > For parseLogLevels() this is not actually true. The function accesses a > > member variable, which you now pass it as a function argument. What's > > the advantage of this patch ? > > Ahh, you're right. The motivation is to decouple the log level specification string > source (the environment variable) and destination (`levels_` list) from the > parsing itself. This was done mostly because I was considering potential solutions > to https://bugs.libcamera.org/show_bug.cgi?id=243. Another thing was the introduction > of the lock in the next patch, keeping the number of places where a mutex protected > object is accessed seems like a good idea (since it is not enforced by any kind > of wrapper type) (although since `parseLogLevels()` is only called from > the constructor, I opted not to do any locking there in the next patch). As the Logger::mutex_ introduced in 8/9 doesn't protect Logger::levels_, and as this series doesn't fix bug #243, I'm tempted to postpone this patch until we address the bug fully. It's hard for me to tell whether or not this patch goes in the right direction without seeing more of the bug being addressed. > > > So move them into an anonymous namespace. > > > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > > --- > > > src/libcamera/base/log.cpp | 186 ++++++++++++++++++------------------- > > > 1 file changed, 93 insertions(+), 93 deletions(-) > > > > > > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp > > > index 0dfdb0e9b..19fc5cc67 100644 > > > --- a/src/libcamera/base/log.cpp > > > +++ b/src/libcamera/base/log.cpp > > > @@ -65,7 +65,9 @@ > > > > > > namespace libcamera { > > > > > > -static int log_severity_to_syslog(LogSeverity severity) > > > +namespace { > > > + > > > +int log_severity_to_syslog(LogSeverity severity) > > > { > > > switch (severity) { > > > case LogDebug: > > > @@ -83,7 +85,7 @@ static int log_severity_to_syslog(LogSeverity severity) > > > } > > > } > > > > > > -static const char *log_severity_name(LogSeverity severity) > > > +const char *log_severity_name(LogSeverity severity) > > > { > > > static const char *const names[] = { > > > "DEBUG", > > > @@ -99,6 +101,92 @@ static const char *log_severity_name(LogSeverity severity) > > > return "UNKWN"; > > > } > > > > > > +/** > > > + * \brief Parse a log level string into a LogSeverity > > > + * \param[in] level The log level string > > > + * > > > + * Log levels can be specified as an integer value in the range from LogDebug to > > > + * LogFatal, or as a string corresponding to the severity name in uppercase. Any > > > + * other value is invalid. > > > + * > > > + * \return The log severity, or LogInvalid if the string is invalid > > > + */ > > > +LogSeverity parseLogLevel(std::string_view level) > > > +{ > > > + static const char *const names[] = { > > > + "DEBUG", > > > + "INFO", > > > + "WARN", > > > + "ERROR", > > > + "FATAL", > > > + }; > > > + > > > + unsigned int severity; > > > + > > > + if (std::isdigit(level[0])) { > > > + auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity); > > > + if (ec != std::errc() || *end != '\0' || severity > LogFatal) > > > + severity = LogInvalid; > > > + } else { > > > + severity = LogInvalid; > > > + for (unsigned int i = 0; i < std::size(names); ++i) { > > > + if (names[i] == level) { > > > + severity = i; > > > + break; > > > + } > > > + } > > > + } > > > + > > > + return static_cast<LogSeverity>(severity); > > > +} > > > + > > > +/** > > > + * \brief Parse the log levels from the environment > > > > Missing \param > > ACK > > > > + * > > > + * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable > > > + * as a list of "category:level" pairs, separated by commas (','). Parse the > > > + * variable and store the levels to configure all log categories. > > > + */ > > > +void parseLogLevels(const char *line, std::list<std::pair<std::string, LogSeverity>> &levels) > > > +{ > > > + for (const char *pair = line; *line != '\0'; pair = line) { > > > + const char *comma = strchrnul(line, ','); > > > + size_t len = comma - pair; > > > + > > > + /* Skip over the comma. */ > > > + line = *comma == ',' ? comma + 1 : comma; > > > + > > > + /* Skip to the next pair if the pair is empty. */ > > > + if (!len) > > > + continue; > > > + > > > + std::string_view category; > > > + std::string_view level; > > > + > > > + const char *colon = static_cast<const char *>(memchr(pair, ':', len)); > > > + if (!colon) { > > > + /* 'x' is a shortcut for '*:x'. */ > > > + category = "*"; > > > + level = std::string_view(pair, len); > > > + } else { > > > + category = std::string_view(pair, colon - pair); > > > + level = std::string_view(colon + 1, comma - colon - 1); > > > + } > > > + > > > + /* Both the category and the level must be specified. */ > > > + if (category.empty() || level.empty()) > > > + continue; > > > + > > > + LogSeverity severity = parseLogLevel(level); > > > + if (severity == LogInvalid) > > > + continue; > > > + > > > + levels.emplace_back(category, severity); > > > + } > > > +} > > > + > > > +} /* namespace */ > > > + > > > /** > > > * \brief Log output > > > * > > > @@ -312,8 +400,6 @@ private: > > > Logger(); > > > > > > void parseLogFile(); > > > - void parseLogLevels(); > > > - static LogSeverity parseLogLevel(std::string_view level); > > > > > > friend LogCategory; > > > void registerCategory(LogCategory *category); > > > @@ -592,7 +678,9 @@ Logger::Logger() > > > logSetStream(&std::cerr, color); > > > > > > parseLogFile(); > > > - parseLogLevels(); > > > + > > > + if (const char *debug = utils::secure_getenv("LIBCAMERA_LOG_LEVELS")) > > > + parseLogLevels(debug, levels_); > > > } > > > > > > /** > > > @@ -618,94 +706,6 @@ void Logger::parseLogFile() > > > logSetFile(file, false); > > > } > > > > > > -/** > > > - * \brief Parse the log levels from the environment > > > - * > > > - * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable > > > - * as a list of "category:level" pairs, separated by commas (','). Parse the > > > - * variable and store the levels to configure all log categories. > > > - */ > > > -void Logger::parseLogLevels() > > > -{ > > > - const char *debug = utils::secure_getenv("LIBCAMERA_LOG_LEVELS"); > > > - if (!debug) > > > - return; > > > - > > > - for (const char *pair = debug; *debug != '\0'; pair = debug) { > > > - const char *comma = strchrnul(debug, ','); > > > - size_t len = comma - pair; > > > - > > > - /* Skip over the comma. */ > > > - debug = *comma == ',' ? comma + 1 : comma; > > > - > > > - /* Skip to the next pair if the pair is empty. */ > > > - if (!len) > > > - continue; > > > - > > > - std::string_view category; > > > - std::string_view level; > > > - > > > - const char *colon = static_cast<const char *>(memchr(pair, ':', len)); > > > - if (!colon) { > > > - /* 'x' is a shortcut for '*:x'. */ > > > - category = "*"; > > > - level = std::string_view(pair, len); > > > - } else { > > > - category = std::string_view(pair, colon - pair); > > > - level = std::string_view(colon + 1, comma - colon - 1); > > > - } > > > - > > > - /* Both the category and the level must be specified. */ > > > - if (category.empty() || level.empty()) > > > - continue; > > > - > > > - LogSeverity severity = parseLogLevel(level); > > > - if (severity == LogInvalid) > > > - continue; > > > - > > > - levels_.emplace_back(category, severity); > > > - } > > > -} > > > - > > > -/** > > > - * \brief Parse a log level string into a LogSeverity > > > - * \param[in] level The log level string > > > - * > > > - * Log levels can be specified as an integer value in the range from LogDebug to > > > - * LogFatal, or as a string corresponding to the severity name in uppercase. Any > > > - * other value is invalid. > > > - * > > > - * \return The log severity, or LogInvalid if the string is invalid > > > - */ > > > -LogSeverity Logger::parseLogLevel(std::string_view level) > > > -{ > > > - static const char *const names[] = { > > > - "DEBUG", > > > - "INFO", > > > - "WARN", > > > - "ERROR", > > > - "FATAL", > > > - }; > > > - > > > - unsigned int severity; > > > - > > > - if (std::isdigit(level[0])) { > > > - auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity); > > > - if (ec != std::errc() || *end != '\0' || severity > LogFatal) > > > - severity = LogInvalid; > > > - } else { > > > - severity = LogInvalid; > > > - for (unsigned int i = 0; i < std::size(names); ++i) { > > > - if (names[i] == level) { > > > - severity = i; > > > - break; > > > - } > > > - } > > > - } > > > - > > > - return static_cast<LogSeverity>(severity); > > > -} > > > - > > > /** > > > * \brief Register a log category with the logger > > > * \param[in] category The log category
diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index 0dfdb0e9b..19fc5cc67 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -65,7 +65,9 @@ namespace libcamera { -static int log_severity_to_syslog(LogSeverity severity) +namespace { + +int log_severity_to_syslog(LogSeverity severity) { switch (severity) { case LogDebug: @@ -83,7 +85,7 @@ static int log_severity_to_syslog(LogSeverity severity) } } -static const char *log_severity_name(LogSeverity severity) +const char *log_severity_name(LogSeverity severity) { static const char *const names[] = { "DEBUG", @@ -99,6 +101,92 @@ static const char *log_severity_name(LogSeverity severity) return "UNKWN"; } +/** + * \brief Parse a log level string into a LogSeverity + * \param[in] level The log level string + * + * Log levels can be specified as an integer value in the range from LogDebug to + * LogFatal, or as a string corresponding to the severity name in uppercase. Any + * other value is invalid. + * + * \return The log severity, or LogInvalid if the string is invalid + */ +LogSeverity parseLogLevel(std::string_view level) +{ + static const char *const names[] = { + "DEBUG", + "INFO", + "WARN", + "ERROR", + "FATAL", + }; + + unsigned int severity; + + if (std::isdigit(level[0])) { + auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity); + if (ec != std::errc() || *end != '\0' || severity > LogFatal) + severity = LogInvalid; + } else { + severity = LogInvalid; + for (unsigned int i = 0; i < std::size(names); ++i) { + if (names[i] == level) { + severity = i; + break; + } + } + } + + return static_cast<LogSeverity>(severity); +} + +/** + * \brief Parse the log levels from the environment + * + * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable + * as a list of "category:level" pairs, separated by commas (','). Parse the + * variable and store the levels to configure all log categories. + */ +void parseLogLevels(const char *line, std::list<std::pair<std::string, LogSeverity>> &levels) +{ + for (const char *pair = line; *line != '\0'; pair = line) { + const char *comma = strchrnul(line, ','); + size_t len = comma - pair; + + /* Skip over the comma. */ + line = *comma == ',' ? comma + 1 : comma; + + /* Skip to the next pair if the pair is empty. */ + if (!len) + continue; + + std::string_view category; + std::string_view level; + + const char *colon = static_cast<const char *>(memchr(pair, ':', len)); + if (!colon) { + /* 'x' is a shortcut for '*:x'. */ + category = "*"; + level = std::string_view(pair, len); + } else { + category = std::string_view(pair, colon - pair); + level = std::string_view(colon + 1, comma - colon - 1); + } + + /* Both the category and the level must be specified. */ + if (category.empty() || level.empty()) + continue; + + LogSeverity severity = parseLogLevel(level); + if (severity == LogInvalid) + continue; + + levels.emplace_back(category, severity); + } +} + +} /* namespace */ + /** * \brief Log output * @@ -312,8 +400,6 @@ private: Logger(); void parseLogFile(); - void parseLogLevels(); - static LogSeverity parseLogLevel(std::string_view level); friend LogCategory; void registerCategory(LogCategory *category); @@ -592,7 +678,9 @@ Logger::Logger() logSetStream(&std::cerr, color); parseLogFile(); - parseLogLevels(); + + if (const char *debug = utils::secure_getenv("LIBCAMERA_LOG_LEVELS")) + parseLogLevels(debug, levels_); } /** @@ -618,94 +706,6 @@ void Logger::parseLogFile() logSetFile(file, false); } -/** - * \brief Parse the log levels from the environment - * - * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable - * as a list of "category:level" pairs, separated by commas (','). Parse the - * variable and store the levels to configure all log categories. - */ -void Logger::parseLogLevels() -{ - const char *debug = utils::secure_getenv("LIBCAMERA_LOG_LEVELS"); - if (!debug) - return; - - for (const char *pair = debug; *debug != '\0'; pair = debug) { - const char *comma = strchrnul(debug, ','); - size_t len = comma - pair; - - /* Skip over the comma. */ - debug = *comma == ',' ? comma + 1 : comma; - - /* Skip to the next pair if the pair is empty. */ - if (!len) - continue; - - std::string_view category; - std::string_view level; - - const char *colon = static_cast<const char *>(memchr(pair, ':', len)); - if (!colon) { - /* 'x' is a shortcut for '*:x'. */ - category = "*"; - level = std::string_view(pair, len); - } else { - category = std::string_view(pair, colon - pair); - level = std::string_view(colon + 1, comma - colon - 1); - } - - /* Both the category and the level must be specified. */ - if (category.empty() || level.empty()) - continue; - - LogSeverity severity = parseLogLevel(level); - if (severity == LogInvalid) - continue; - - levels_.emplace_back(category, severity); - } -} - -/** - * \brief Parse a log level string into a LogSeverity - * \param[in] level The log level string - * - * Log levels can be specified as an integer value in the range from LogDebug to - * LogFatal, or as a string corresponding to the severity name in uppercase. Any - * other value is invalid. - * - * \return The log severity, or LogInvalid if the string is invalid - */ -LogSeverity Logger::parseLogLevel(std::string_view level) -{ - static const char *const names[] = { - "DEBUG", - "INFO", - "WARN", - "ERROR", - "FATAL", - }; - - unsigned int severity; - - if (std::isdigit(level[0])) { - auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity); - if (ec != std::errc() || *end != '\0' || severity > LogFatal) - severity = LogInvalid; - } else { - severity = LogInvalid; - for (unsigned int i = 0; i < std::size(names); ++i) { - if (names[i] == level) { - severity = i; - break; - } - } - } - - return static_cast<LogSeverity>(severity); -} - /** * \brief Register a log category with the logger * \param[in] category The log category
These two functions do not need to be exposed in any way, nor do they need to be a member of the `Logger` class. So move them into an anonymous namespace. Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> --- src/libcamera/base/log.cpp | 186 ++++++++++++++++++------------------- 1 file changed, 93 insertions(+), 93 deletions(-)