[{"id":33266,"web_url":"https://patchwork.libcamera.org/comment/33266/","msgid":"","date":"2025-02-03T17:02:01","subject":"Re: [RFC PATCH v2 7/9] libcamera: base: log: Split\n\t`parseLogLevel[s]()`","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Thu, Jan 30, 2025 at 07:58:52PM +0000, Barnabás Pőcze wrote:\n> These two functions do not need to be exposed in any\n> way, nor do they need to be a member of the `Logger` class.\n>\n> So move them into an anonymous namespace.\n>\n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/libcamera/base/log.cpp | 186 ++++++++++++++++++-------------------\n> 1 file changed, 93 insertions(+), 93 deletions(-)\n>\n> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> index 0dfdb0e9b..19fc5cc67 100644\n> --- a/src/libcamera/base/log.cpp\n> +++ b/src/libcamera/base/log.cpp\n> @@ -65,7 +65,9 @@\n>\n> namespace libcamera {\n>\n> -static int log_severity_to_syslog(LogSeverity severity)\n> +namespace {\n> +\n> +int log_severity_to_syslog(LogSeverity severity)\n> {\n> \tswitch (severity) {\n> \tcase LogDebug:\n> @@ -83,7 +85,7 @@ static int log_severity_to_syslog(LogSeverity severity)\n> \t}\n> }\n>\n> -static const char *log_severity_name(LogSeverity severity)\n> +const char *log_severity_name(LogSeverity severity)\n> {\n> \tstatic const char *const names[] = {\n> \t\t\"DEBUG\",\n> @@ -99,6 +101,92 @@ static const char *log_severity_name(LogSeverity severity)\n> \t\treturn \"UNKWN\";\n> }\n>\n> +/**\n\nMaybe they don't need to be doxygened anymore ?\n\nNot a big deal\nReviewed-by: Jacopo Mondi \n\n> + * \\brief Parse a log level string into a LogSeverity\n> + * \\param[in] level The log level string\n> + *\n> + * Log levels can be specified as an integer value in the range from LogDebug to\n> + * LogFatal, or as a string corresponding to the severity name in uppercase. Any\n> + * other value is invalid.\n> + *\n> + * \\return The log severity, or LogInvalid if the string is invalid\n> + */\n> +LogSeverity parseLogLevel(std::string_view level)\n> +{\n> +\tstatic const char *const names[] = {\n> +\t\t\"DEBUG\",\n> +\t\t\"INFO\",\n> +\t\t\"WARN\",\n> +\t\t\"ERROR\",\n> +\t\t\"FATAL\",\n> +\t};\n> +\n> +\tunsigned int severity;\n> +\n> +\tif (std::isdigit(level[0])) {\n> +\t\tauto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);\n> +\t\tif (ec != std::errc() || *end != '\\0' || severity > LogFatal)\n> +\t\t\tseverity = LogInvalid;\n> +\t} else {\n> +\t\tseverity = LogInvalid;\n> +\t\tfor (unsigned int i = 0; i < std::size(names); ++i) {\n> +\t\t\tif (names[i] == level) {\n> +\t\t\t\tseverity = i;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> +\treturn static_cast(severity);\n> +}\n> +\n> +/**\n> + * \\brief Parse the log levels from the environment\n> + *\n> + * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable\n> + * as a list of \"category:level\" pairs, separated by commas (','). Parse the\n> + * variable and store the levels to configure all log categories.\n> + */\n> +void parseLogLevels(const char *line, std::list> &levels)\n> +{\n> +\tfor (const char *pair = line; *line != '\\0'; pair = line) {\n> +\t\tconst char *comma = strchrnul(line, ',');\n> +\t\tsize_t len = comma - pair;\n> +\n> +\t\t/* Skip over the comma. */\n> +\t\tline = *comma == ',' ? comma + 1 : comma;\n> +\n> +\t\t/* Skip to the next pair if the pair is empty. */\n> +\t\tif (!len)\n> +\t\t\tcontinue;\n> +\n> +\t\tstd::string_view category;\n> +\t\tstd::string_view level;\n> +\n> +\t\tconst char *colon = static_cast(memchr(pair, ':', len));\n> +\t\tif (!colon) {\n> +\t\t\t/* 'x' is a shortcut for '*:x'. */\n> +\t\t\tcategory = \"*\";\n> +\t\t\tlevel = std::string_view(pair, len);\n> +\t\t} else {\n> +\t\t\tcategory = std::string_view(pair, colon - pair);\n> +\t\t\tlevel = std::string_view(colon + 1, comma - colon - 1);\n> +\t\t}\n> +\n> +\t\t/* Both the category and the level must be specified. */\n> +\t\tif (category.empty() || level.empty())\n> +\t\t\tcontinue;\n> +\n> +\t\tLogSeverity severity = parseLogLevel(level);\n> +\t\tif (severity == LogInvalid)\n> +\t\t\tcontinue;\n> +\n> +\t\tlevels.emplace_back(category, severity);\n> +\t}\n> +}\n> +\n> +} /* namespace */\n> +\n> /**\n> * \\brief Log output\n> *\n> @@ -312,8 +400,6 @@ private:\n> \tLogger();\n>\n> \tvoid parseLogFile();\n> -\tvoid parseLogLevels();\n> -\tstatic LogSeverity parseLogLevel(std::string_view level);\n>\n> \tfriend LogCategory;\n> \tvoid registerCategory(LogCategory *category);\n> @@ -592,7 +678,9 @@ Logger::Logger()\n> \tlogSetStream(&std::cerr, color);\n>\n> \tparseLogFile();\n> -\tparseLogLevels();\n> +\n> +\tif (const char *debug = utils::secure_getenv(\"LIBCAMERA_LOG_LEVELS\"))\n> +\t\tparseLogLevels(debug, levels_);\n> }\n>\n> /**\n> @@ -618,94 +706,6 @@ void Logger::parseLogFile()\n> \tlogSetFile(file, false);\n> }\n>\n> -/**\n> - * \\brief Parse the log levels from the environment\n> - *\n> - * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable\n> - * as a list of \"category:level\" pairs, separated by commas (','). Parse the\n> - * variable and store the levels to configure all log categories.\n> - */\n> -void Logger::parseLogLevels()\n> -{\n> -\tconst char *debug = utils::secure_getenv(\"LIBCAMERA_LOG_LEVELS\");\n> -\tif (!debug)\n> -\t\treturn;\n> -\n> -\tfor (const char *pair = debug; *debug != '\\0'; pair = debug) {\n> -\t\tconst char *comma = strchrnul(debug, ',');\n> -\t\tsize_t len = comma - pair;\n> -\n> -\t\t/* Skip over the comma. */\n> -\t\tdebug = *comma == ',' ? comma + 1 : comma;\n> -\n> -\t\t/* Skip to the next pair if the pair is empty. */\n> -\t\tif (!len)\n> -\t\t\tcontinue;\n> -\n> -\t\tstd::string_view category;\n> -\t\tstd::string_view level;\n> -\n> -\t\tconst char *colon = static_cast(memchr(pair, ':', len));\n> -\t\tif (!colon) {\n> -\t\t\t/* 'x' is a shortcut for '*:x'. */\n> -\t\t\tcategory = \"*\";\n> -\t\t\tlevel = std::string_view(pair, len);\n> -\t\t} else {\n> -\t\t\tcategory = std::string_view(pair, colon - pair);\n> -\t\t\tlevel = std::string_view(colon + 1, comma - colon - 1);\n> -\t\t}\n> -\n> -\t\t/* Both the category and the level must be specified. */\n> -\t\tif (category.empty() || level.empty())\n> -\t\t\tcontinue;\n> -\n> -\t\tLogSeverity severity = parseLogLevel(level);\n> -\t\tif (severity == LogInvalid)\n> -\t\t\tcontinue;\n> -\n> -\t\tlevels_.emplace_back(category, severity);\n> -\t}\n> -}\n> -\n> -/**\n> - * \\brief Parse a log level string into a LogSeverity\n> - * \\param[in] level The log level string\n> - *\n> - * Log levels can be specified as an integer value in the range from LogDebug to\n> - * LogFatal, or as a string corresponding to the severity name in uppercase. Any\n> - * other value is invalid.\n> - *\n> - * \\return The log severity, or LogInvalid if the string is invalid\n> - */\n> -LogSeverity Logger::parseLogLevel(std::string_view level)\n> -{\n> -\tstatic const char *const names[] = {\n> -\t\t\"DEBUG\",\n> -\t\t\"INFO\",\n> -\t\t\"WARN\",\n> -\t\t\"ERROR\",\n> -\t\t\"FATAL\",\n> -\t};\n> -\n> -\tunsigned int severity;\n> -\n> -\tif (std::isdigit(level[0])) {\n> -\t\tauto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);\n> -\t\tif (ec != std::errc() || *end != '\\0' || severity > LogFatal)\n> -\t\t\tseverity = LogInvalid;\n> -\t} else {\n> -\t\tseverity = LogInvalid;\n> -\t\tfor (unsigned int i = 0; i < std::size(names); ++i) {\n> -\t\t\tif (names[i] == level) {\n> -\t\t\t\tseverity = i;\n> -\t\t\t\tbreak;\n> -\t\t\t}\n> -\t\t}\n> -\t}\n> -\n> -\treturn static_cast(severity);\n> -}\n> -\n> /**\n> * \\brief Register a log category with the logger\n> * \\param[in] category The log category\n> --\n> 2.48.1\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0E67EBD80A\n\tfor ;\n\tMon, 3 Feb 2025 17:02:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F82668598;\n\tMon, 3 Feb 2025 18:02:06 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5D04661876\n\tfor ;\n\tMon, 3 Feb 2025 18:02:05 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 95203497;\n\tMon, 3 Feb 2025 18:00:53 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"KbEYcP+Z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1738602053;\n\tbh=j9JiqMMF8Pzo/YNFE803ajF1j9n1DAF+D9UHNbnV9Wo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KbEYcP+ZQ0iOyc4l5cS/n/3g+TYDvHzSYmNNebHsIfBjbUxg7Up1BfvMUbAO8/7Bm\n\tq+88Cn7aAFzv9Cg6IO40mSIh0ye7UBwGQmFQbWEjlZ8Q1mAmigLdFOeiEOS55Mj2tl\n\tk6Yo0vdeYjNIk47yMhXtFovoGGNjmd8hhY67BF/0=","Date":"Mon, 3 Feb 2025 18:02:01 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 7/9] libcamera: base: log: Split\n\t`parseLogLevel[s]()`","Message-ID":"","References":"<20250130195811.1230581-1-pobrn@protonmail.com>\n\t<20250130195811.1230581-8-pobrn@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250130195811.1230581-8-pobrn@protonmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33306,"web_url":"https://patchwork.libcamera.org/comment/33306/","msgid":"<20250206164848.GA12400@pendragon.ideasonboard.com>","date":"2025-02-06T16:48:48","subject":"Re: [RFC PATCH v2 7/9] libcamera: base: log: Split\n\t`parseLogLevel[s]()`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnab@s,\n\nThank you for the patch.\n\nOn Thu, Jan 30, 2025 at 07:58:52PM +0000, Barnabás Pőcze wrote:\n> These two functions do not need to be exposed in any\n> way, nor do they need to be a member of the `Logger` class.\n\nFor parseLogLevels() this is not actually true. The function accesses a\nmember variable, which you now pass it as a function argument. What's\nthe advantage of this patch ?\n\n> So move them into an anonymous namespace.\n> \n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/libcamera/base/log.cpp | 186 ++++++++++++++++++-------------------\n> 1 file changed, 93 insertions(+), 93 deletions(-)\n> \n> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> index 0dfdb0e9b..19fc5cc67 100644\n> --- a/src/libcamera/base/log.cpp\n> +++ b/src/libcamera/base/log.cpp\n> @@ -65,7 +65,9 @@\n> \n> namespace libcamera {\n> \n> -static int log_severity_to_syslog(LogSeverity severity)\n> +namespace {\n> +\n> +int log_severity_to_syslog(LogSeverity severity)\n> {\n> \tswitch (severity) {\n> \tcase LogDebug:\n> @@ -83,7 +85,7 @@ static int log_severity_to_syslog(LogSeverity severity)\n> \t}\n> }\n> \n> -static const char *log_severity_name(LogSeverity severity)\n> +const char *log_severity_name(LogSeverity severity)\n> {\n> \tstatic const char *const names[] = {\n> \t\t\"DEBUG\",\n> @@ -99,6 +101,92 @@ static const char *log_severity_name(LogSeverity severity)\n> \t\treturn \"UNKWN\";\n> }\n> \n> +/**\n> + * \\brief Parse a log level string into a LogSeverity\n> + * \\param[in] level The log level string\n> + *\n> + * Log levels can be specified as an integer value in the range from LogDebug to\n> + * LogFatal, or as a string corresponding to the severity name in uppercase. Any\n> + * other value is invalid.\n> + *\n> + * \\return The log severity, or LogInvalid if the string is invalid\n> + */\n> +LogSeverity parseLogLevel(std::string_view level)\n> +{\n> +\tstatic const char *const names[] = {\n> +\t\t\"DEBUG\",\n> +\t\t\"INFO\",\n> +\t\t\"WARN\",\n> +\t\t\"ERROR\",\n> +\t\t\"FATAL\",\n> +\t};\n> +\n> +\tunsigned int severity;\n> +\n> +\tif (std::isdigit(level[0])) {\n> +\t\tauto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);\n> +\t\tif (ec != std::errc() || *end != '\\0' || severity > LogFatal)\n> +\t\t\tseverity = LogInvalid;\n> +\t} else {\n> +\t\tseverity = LogInvalid;\n> +\t\tfor (unsigned int i = 0; i < std::size(names); ++i) {\n> +\t\t\tif (names[i] == level) {\n> +\t\t\t\tseverity = i;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> +\treturn static_cast(severity);\n> +}\n> +\n> +/**\n> + * \\brief Parse the log levels from the environment\n\nMissing \\param\n\n> + *\n> + * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable\n> + * as a list of \"category:level\" pairs, separated by commas (','). Parse the\n> + * variable and store the levels to configure all log categories.\n> + */\n> +void parseLogLevels(const char *line, std::list> &levels)\n> +{\n> +\tfor (const char *pair = line; *line != '\\0'; pair = line) {\n> +\t\tconst char *comma = strchrnul(line, ',');\n> +\t\tsize_t len = comma - pair;\n> +\n> +\t\t/* Skip over the comma. */\n> +\t\tline = *comma == ',' ? comma + 1 : comma;\n> +\n> +\t\t/* Skip to the next pair if the pair is empty. */\n> +\t\tif (!len)\n> +\t\t\tcontinue;\n> +\n> +\t\tstd::string_view category;\n> +\t\tstd::string_view level;\n> +\n> +\t\tconst char *colon = static_cast(memchr(pair, ':', len));\n> +\t\tif (!colon) {\n> +\t\t\t/* 'x' is a shortcut for '*:x'. */\n> +\t\t\tcategory = \"*\";\n> +\t\t\tlevel = std::string_view(pair, len);\n> +\t\t} else {\n> +\t\t\tcategory = std::string_view(pair, colon - pair);\n> +\t\t\tlevel = std::string_view(colon + 1, comma - colon - 1);\n> +\t\t}\n> +\n> +\t\t/* Both the category and the level must be specified. */\n> +\t\tif (category.empty() || level.empty())\n> +\t\t\tcontinue;\n> +\n> +\t\tLogSeverity severity = parseLogLevel(level);\n> +\t\tif (severity == LogInvalid)\n> +\t\t\tcontinue;\n> +\n> +\t\tlevels.emplace_back(category, severity);\n> +\t}\n> +}\n> +\n> +} /* namespace */\n> +\n> /**\n> * \\brief Log output\n> *\n> @@ -312,8 +400,6 @@ private:\n> \tLogger();\n> \n> \tvoid parseLogFile();\n> -\tvoid parseLogLevels();\n> -\tstatic LogSeverity parseLogLevel(std::string_view level);\n> \n> \tfriend LogCategory;\n> \tvoid registerCategory(LogCategory *category);\n> @@ -592,7 +678,9 @@ Logger::Logger()\n> \tlogSetStream(&std::cerr, color);\n> \n> \tparseLogFile();\n> -\tparseLogLevels();\n> +\n> +\tif (const char *debug = utils::secure_getenv(\"LIBCAMERA_LOG_LEVELS\"))\n> +\t\tparseLogLevels(debug, levels_);\n> }\n> \n> /**\n> @@ -618,94 +706,6 @@ void Logger::parseLogFile()\n> \tlogSetFile(file, false);\n> }\n> \n> -/**\n> - * \\brief Parse the log levels from the environment\n> - *\n> - * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable\n> - * as a list of \"category:level\" pairs, separated by commas (','). Parse the\n> - * variable and store the levels to configure all log categories.\n> - */\n> -void Logger::parseLogLevels()\n> -{\n> -\tconst char *debug = utils::secure_getenv(\"LIBCAMERA_LOG_LEVELS\");\n> -\tif (!debug)\n> -\t\treturn;\n> -\n> -\tfor (const char *pair = debug; *debug != '\\0'; pair = debug) {\n> -\t\tconst char *comma = strchrnul(debug, ',');\n> -\t\tsize_t len = comma - pair;\n> -\n> -\t\t/* Skip over the comma. */\n> -\t\tdebug = *comma == ',' ? comma + 1 : comma;\n> -\n> -\t\t/* Skip to the next pair if the pair is empty. */\n> -\t\tif (!len)\n> -\t\t\tcontinue;\n> -\n> -\t\tstd::string_view category;\n> -\t\tstd::string_view level;\n> -\n> -\t\tconst char *colon = static_cast(memchr(pair, ':', len));\n> -\t\tif (!colon) {\n> -\t\t\t/* 'x' is a shortcut for '*:x'. */\n> -\t\t\tcategory = \"*\";\n> -\t\t\tlevel = std::string_view(pair, len);\n> -\t\t} else {\n> -\t\t\tcategory = std::string_view(pair, colon - pair);\n> -\t\t\tlevel = std::string_view(colon + 1, comma - colon - 1);\n> -\t\t}\n> -\n> -\t\t/* Both the category and the level must be specified. */\n> -\t\tif (category.empty() || level.empty())\n> -\t\t\tcontinue;\n> -\n> -\t\tLogSeverity severity = parseLogLevel(level);\n> -\t\tif (severity == LogInvalid)\n> -\t\t\tcontinue;\n> -\n> -\t\tlevels_.emplace_back(category, severity);\n> -\t}\n> -}\n> -\n> -/**\n> - * \\brief Parse a log level string into a LogSeverity\n> - * \\param[in] level The log level string\n> - *\n> - * Log levels can be specified as an integer value in the range from LogDebug to\n> - * LogFatal, or as a string corresponding to the severity name in uppercase. Any\n> - * other value is invalid.\n> - *\n> - * \\return The log severity, or LogInvalid if the string is invalid\n> - */\n> -LogSeverity Logger::parseLogLevel(std::string_view level)\n> -{\n> -\tstatic const char *const names[] = {\n> -\t\t\"DEBUG\",\n> -\t\t\"INFO\",\n> -\t\t\"WARN\",\n> -\t\t\"ERROR\",\n> -\t\t\"FATAL\",\n> -\t};\n> -\n> -\tunsigned int severity;\n> -\n> -\tif (std::isdigit(level[0])) {\n> -\t\tauto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);\n> -\t\tif (ec != std::errc() || *end != '\\0' || severity > LogFatal)\n> -\t\t\tseverity = LogInvalid;\n> -\t} else {\n> -\t\tseverity = LogInvalid;\n> -\t\tfor (unsigned int i = 0; i < std::size(names); ++i) {\n> -\t\t\tif (names[i] == level) {\n> -\t\t\t\tseverity = i;\n> -\t\t\t\tbreak;\n> -\t\t\t}\n> -\t\t}\n> -\t}\n> -\n> -\treturn static_cast(severity);\n> -}\n> -\n> /**\n> * \\brief Register a log category with the logger\n> * \\param[in] category The log category","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 55C18C32EA\n\tfor ;\n\tThu, 6 Feb 2025 16:48:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4D34E685EB;\n\tThu, 6 Feb 2025 17:48:55 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B37A96053F\n\tfor ;\n\tThu, 6 Feb 2025 17:48:53 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B0DBE11DA;\n\tThu, 6 Feb 2025 17:47:39 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lxef8wra\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1738860459;\n\tbh=VgVDpFsApHcNtOWVWySDnJe7dddE6igRFeuE+XpT/VI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lxef8wraufHpz/NbzKIIgFZutJjGVVD9JfjK6G/1uXkUV9eCzD8d10yB4UwJurBCP\n\tuNcXDWFf2mBdcup7MlW/EJ7gFBGCsqTFiNVrn+TaLyQXM3Kf5sfffoHIbJzP+DUGtq\n\tC/rQDSh7pJRs35plv7bW22rniXIleJfclurw/oH0=","Date":"Thu, 6 Feb 2025 18:48:48 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 7/9] libcamera: base: log: Split\n\t`parseLogLevel[s]()`","Message-ID":"<20250206164848.GA12400@pendragon.ideasonboard.com>","References":"<20250130195811.1230581-1-pobrn@protonmail.com>\n\t<20250130195811.1230581-8-pobrn@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250130195811.1230581-8-pobrn@protonmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33311,"web_url":"https://patchwork.libcamera.org/comment/33311/","msgid":"","date":"2025-02-06T17:18:55","subject":"Re: [RFC PATCH v2 7/9] libcamera: base: log: Split\n\t`parseLogLevel[s]()`","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2025. február 6., csütörtök 17:48 keltezéssel, Laurent Pinchart írta:\n\n> Hi Barnab@s,\n> \n> Thank you for the patch.\n> \n> On Thu, Jan 30, 2025 at 07:58:52PM +0000, Barnabás Pőcze wrote:\n> > These two functions do not need to be exposed in any\n> > way, nor do they need to be a member of the `Logger` class.\n> \n> For parseLogLevels() this is not actually true. The function accesses a\n> member variable, which you now pass it as a function argument. What's\n> the advantage of this patch ?\n\nAhh, you're right. The motivation is to decouple the log level specification string\nsource (the environment variable) and destination (`levels_` list) from the\nparsing itself. This was done mostly because I was considering potential solutions\nto https://bugs.libcamera.org/show_bug.cgi?id=243. Another thing was the introduction\nof the lock in the next patch, keeping the number of places where a mutex protected\nobject is accessed seems like a good idea (since it is not enforced by any kind\nof wrapper type) (although since `parseLogLevels()` is only called from\nthe constructor, I opted not to do any locking there in the next patch).\n\n\n> \n> > So move them into an anonymous namespace.\n> >\n> > Signed-off-by: Barnabás Pőcze \n> > ---\n> > src/libcamera/base/log.cpp | 186 ++++++++++++++++++-------------------\n> > 1 file changed, 93 insertions(+), 93 deletions(-)\n> >\n> > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> > index 0dfdb0e9b..19fc5cc67 100644\n> > --- a/src/libcamera/base/log.cpp\n> > +++ b/src/libcamera/base/log.cpp\n> > @@ -65,7 +65,9 @@\n> >\n> > namespace libcamera {\n> >\n> > -static int log_severity_to_syslog(LogSeverity severity)\n> > +namespace {\n> > +\n> > +int log_severity_to_syslog(LogSeverity severity)\n> > {\n> > \tswitch (severity) {\n> > \tcase LogDebug:\n> > @@ -83,7 +85,7 @@ static int log_severity_to_syslog(LogSeverity severity)\n> > \t}\n> > }\n> >\n> > -static const char *log_severity_name(LogSeverity severity)\n> > +const char *log_severity_name(LogSeverity severity)\n> > {\n> > \tstatic const char *const names[] = {\n> > \t\t\"DEBUG\",\n> > @@ -99,6 +101,92 @@ static const char *log_severity_name(LogSeverity severity)\n> > \t\treturn \"UNKWN\";\n> > }\n> >\n> > +/**\n> > + * \\brief Parse a log level string into a LogSeverity\n> > + * \\param[in] level The log level string\n> > + *\n> > + * Log levels can be specified as an integer value in the range from LogDebug to\n> > + * LogFatal, or as a string corresponding to the severity name in uppercase. Any\n> > + * other value is invalid.\n> > + *\n> > + * \\return The log severity, or LogInvalid if the string is invalid\n> > + */\n> > +LogSeverity parseLogLevel(std::string_view level)\n> > +{\n> > +\tstatic const char *const names[] = {\n> > +\t\t\"DEBUG\",\n> > +\t\t\"INFO\",\n> > +\t\t\"WARN\",\n> > +\t\t\"ERROR\",\n> > +\t\t\"FATAL\",\n> > +\t};\n> > +\n> > +\tunsigned int severity;\n> > +\n> > +\tif (std::isdigit(level[0])) {\n> > +\t\tauto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);\n> > +\t\tif (ec != std::errc() || *end != '\\0' || severity > LogFatal)\n> > +\t\t\tseverity = LogInvalid;\n> > +\t} else {\n> > +\t\tseverity = LogInvalid;\n> > +\t\tfor (unsigned int i = 0; i < std::size(names); ++i) {\n> > +\t\t\tif (names[i] == level) {\n> > +\t\t\t\tseverity = i;\n> > +\t\t\t\tbreak;\n> > +\t\t\t}\n> > +\t\t}\n> > +\t}\n> > +\n> > +\treturn static_cast(severity);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Parse the log levels from the environment\n> \n> Missing \\param\n\n\nACK\n\n\n> \n> > + *\n> > + * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable\n> > + * as a list of \"category:level\" pairs, separated by commas (','). Parse the\n> > + * variable and store the levels to configure all log categories.\n> > + */\n> > +void parseLogLevels(const char *line, std::list> &levels)\n> > +{\n> > +\tfor (const char *pair = line; *line != '\\0'; pair = line) {\n> > +\t\tconst char *comma = strchrnul(line, ',');\n> > +\t\tsize_t len = comma - pair;\n> > +\n> > +\t\t/* Skip over the comma. */\n> > +\t\tline = *comma == ',' ? comma + 1 : comma;\n> > +\n> > +\t\t/* Skip to the next pair if the pair is empty. */\n> > +\t\tif (!len)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tstd::string_view category;\n> > +\t\tstd::string_view level;\n> > +\n> > +\t\tconst char *colon = static_cast(memchr(pair, ':', len));\n> > +\t\tif (!colon) {\n> > +\t\t\t/* 'x' is a shortcut for '*:x'. */\n> > +\t\t\tcategory = \"*\";\n> > +\t\t\tlevel = std::string_view(pair, len);\n> > +\t\t} else {\n> > +\t\t\tcategory = std::string_view(pair, colon - pair);\n> > +\t\t\tlevel = std::string_view(colon + 1, comma - colon - 1);\n> > +\t\t}\n> > +\n> > +\t\t/* Both the category and the level must be specified. */\n> > +\t\tif (category.empty() || level.empty())\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tLogSeverity severity = parseLogLevel(level);\n> > +\t\tif (severity == LogInvalid)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tlevels.emplace_back(category, severity);\n> > +\t}\n> > +}\n> > +\n> > +} /* namespace */\n> > +\n> > /**\n> > * \\brief Log output\n> > *\n> > @@ -312,8 +400,6 @@ private:\n> > \tLogger();\n> >\n> > \tvoid parseLogFile();\n> > -\tvoid parseLogLevels();\n> > -\tstatic LogSeverity parseLogLevel(std::string_view level);\n> >\n> > \tfriend LogCategory;\n> > \tvoid registerCategory(LogCategory *category);\n> > @@ -592,7 +678,9 @@ Logger::Logger()\n> > \tlogSetStream(&std::cerr, color);\n> >\n> > \tparseLogFile();\n> > -\tparseLogLevels();\n> > +\n> > +\tif (const char *debug = utils::secure_getenv(\"LIBCAMERA_LOG_LEVELS\"))\n> > +\t\tparseLogLevels(debug, levels_);\n> > }\n> >\n> > /**\n> > @@ -618,94 +706,6 @@ void Logger::parseLogFile()\n> > \tlogSetFile(file, false);\n> > }\n> >\n> > -/**\n> > - * \\brief Parse the log levels from the environment\n> > - *\n> > - * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable\n> > - * as a list of \"category:level\" pairs, separated by commas (','). Parse the\n> > - * variable and store the levels to configure all log categories.\n> > - */\n> > -void Logger::parseLogLevels()\n> > -{\n> > -\tconst char *debug = utils::secure_getenv(\"LIBCAMERA_LOG_LEVELS\");\n> > -\tif (!debug)\n> > -\t\treturn;\n> > -\n> > -\tfor (const char *pair = debug; *debug != '\\0'; pair = debug) {\n> > -\t\tconst char *comma = strchrnul(debug, ',');\n> > -\t\tsize_t len = comma - pair;\n> > -\n> > -\t\t/* Skip over the comma. */\n> > -\t\tdebug = *comma == ',' ? comma + 1 : comma;\n> > -\n> > -\t\t/* Skip to the next pair if the pair is empty. */\n> > -\t\tif (!len)\n> > -\t\t\tcontinue;\n> > -\n> > -\t\tstd::string_view category;\n> > -\t\tstd::string_view level;\n> > -\n> > -\t\tconst char *colon = static_cast(memchr(pair, ':', len));\n> > -\t\tif (!colon) {\n> > -\t\t\t/* 'x' is a shortcut for '*:x'. */\n> > -\t\t\tcategory = \"*\";\n> > -\t\t\tlevel = std::string_view(pair, len);\n> > -\t\t} else {\n> > -\t\t\tcategory = std::string_view(pair, colon - pair);\n> > -\t\t\tlevel = std::string_view(colon + 1, comma - colon - 1);\n> > -\t\t}\n> > -\n> > -\t\t/* Both the category and the level must be specified. */\n> > -\t\tif (category.empty() || level.empty())\n> > -\t\t\tcontinue;\n> > -\n> > -\t\tLogSeverity severity = parseLogLevel(level);\n> > -\t\tif (severity == LogInvalid)\n> > -\t\t\tcontinue;\n> > -\n> > -\t\tlevels_.emplace_back(category, severity);\n> > -\t}\n> > -}\n> > -\n> > -/**\n> > - * \\brief Parse a log level string into a LogSeverity\n> > - * \\param[in] level The log level string\n> > - *\n> > - * Log levels can be specified as an integer value in the range from LogDebug to\n> > - * LogFatal, or as a string corresponding to the severity name in uppercase. Any\n> > - * other value is invalid.\n> > - *\n> > - * \\return The log severity, or LogInvalid if the string is invalid\n> > - */\n> > -LogSeverity Logger::parseLogLevel(std::string_view level)\n> > -{\n> > -\tstatic const char *const names[] = {\n> > -\t\t\"DEBUG\",\n> > -\t\t\"INFO\",\n> > -\t\t\"WARN\",\n> > -\t\t\"ERROR\",\n> > -\t\t\"FATAL\",\n> > -\t};\n> > -\n> > -\tunsigned int severity;\n> > -\n> > -\tif (std::isdigit(level[0])) {\n> > -\t\tauto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);\n> > -\t\tif (ec != std::errc() || *end != '\\0' || severity > LogFatal)\n> > -\t\t\tseverity = LogInvalid;\n> > -\t} else {\n> > -\t\tseverity = LogInvalid;\n> > -\t\tfor (unsigned int i = 0; i < std::size(names); ++i) {\n> > -\t\t\tif (names[i] == level) {\n> > -\t\t\t\tseverity = i;\n> > -\t\t\t\tbreak;\n> > -\t\t\t}\n> > -\t\t}\n> > -\t}\n> > -\n> > -\treturn static_cast(severity);\n> > -}\n> > -\n> > /**\n> > * \\brief Register a log category with the logger\n> > * \\param[in] category The log category\n> \n> --\n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B7032C32EF\n\tfor ;\n\tThu, 6 Feb 2025 17:19:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1E5E685F6;\n\tThu, 6 Feb 2025 18:19:01 +0100 (CET)","from mail-10630.protonmail.ch (mail-10630.protonmail.ch\n\t[79.135.106.30])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A1453685E6\n\tfor ;\n\tThu, 6 Feb 2025 18:19:00 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"qrynLv2F\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1738862339; x=1739121539;\n\tbh=tU71NvaC6zQoOeI1leoMp2Y7dODoOcOenbybpT6lE2o=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;\n\tb=qrynLv2FdLYYvEVWsgD1GJ0UISam20+NiJm9zfjnSsvqKkzJVoXq6Q9HeC5pEW+LP\n\t9IeHIjpMG45cvL1wzQi1LT28NCrEHNe3zOaqQBHWFihi3ETD4/g+u0AByHCMsCtqAB\n\tGjfD1xDie+ADqwkUC1wQBFT9K6uTM26a71G2KC5m4dS0BMMlYHPpndOxemOPzduYKi\n\tvpKlbbsLHXJa9gA1X/iebvJDK/J+OH8hB4jG3M/sjnOK1x7LVryqF/wLEY7Bu1Od+0\n\tcunnY4x3zU/6K79P+qh+ZcN+v3PZHqsAkXoXtafPZQmbwgD0B2q0US7SmSsI/p9NbI\n\tRHB2gt+PCPC0Q==","Date":"Thu, 06 Feb 2025 17:18:55 +0000","To":"Laurent Pinchart ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 7/9] libcamera: base: log: Split\n\t`parseLogLevel[s]()`","Message-ID":"","In-Reply-To":"<20250206164848.GA12400@pendragon.ideasonboard.com>","References":"<20250130195811.1230581-1-pobrn@protonmail.com>\n\t<20250130195811.1230581-8-pobrn@protonmail.com>\n\t<20250206164848.GA12400@pendragon.ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"503a195cdb59abfd7a770667af2c96ba1f25e0d4","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33313,"web_url":"https://patchwork.libcamera.org/comment/33313/","msgid":"<20250206172557.GD12400@pendragon.ideasonboard.com>","date":"2025-02-06T17:25:57","subject":"Re: [RFC PATCH v2 7/9] libcamera: base: log: Split\n\t`parseLogLevel[s]()`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Feb 06, 2025 at 05:18:55PM +0000, Barnabás Pőcze wrote:\n> 2025. február 6., csütörtök 17:48 keltezéssel, Laurent Pinchart írta:\n> > On Thu, Jan 30, 2025 at 07:58:52PM +0000, Barnabás Pőcze wrote:\n> > > These two functions do not need to be exposed in any\n> > > way, nor do they need to be a member of the `Logger` class.\n> > \n> > For parseLogLevels() this is not actually true. The function accesses a\n> > member variable, which you now pass it as a function argument. What's\n> > the advantage of this patch ?\n> \n> Ahh, you're right. The motivation is to decouple the log level specification string\n> source (the environment variable) and destination (`levels_` list) from the\n> parsing itself. This was done mostly because I was considering potential solutions\n> to https://bugs.libcamera.org/show_bug.cgi?id=243. Another thing was the introduction\n> of the lock in the next patch, keeping the number of places where a mutex protected\n> object is accessed seems like a good idea (since it is not enforced by any kind\n> of wrapper type) (although since `parseLogLevels()` is only called from\n> the constructor, I opted not to do any locking there in the next patch).\n\nAs the Logger::mutex_ introduced in 8/9 doesn't protect Logger::levels_,\nand as this series doesn't fix bug #243, I'm tempted to postpone this\npatch until we address the bug fully. It's hard for me to tell whether\nor not this patch goes in the right direction without seeing more of the\nbug being addressed.\n\n> > > So move them into an anonymous namespace.\n> > >\n> > > Signed-off-by: Barnabás Pőcze \n> > > ---\n> > > src/libcamera/base/log.cpp | 186 ++++++++++++++++++-------------------\n> > > 1 file changed, 93 insertions(+), 93 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> > > index 0dfdb0e9b..19fc5cc67 100644\n> > > --- a/src/libcamera/base/log.cpp\n> > > +++ b/src/libcamera/base/log.cpp\n> > > @@ -65,7 +65,9 @@\n> > >\n> > > namespace libcamera {\n> > >\n> > > -static int log_severity_to_syslog(LogSeverity severity)\n> > > +namespace {\n> > > +\n> > > +int log_severity_to_syslog(LogSeverity severity)\n> > > {\n> > > \tswitch (severity) {\n> > > \tcase LogDebug:\n> > > @@ -83,7 +85,7 @@ static int log_severity_to_syslog(LogSeverity severity)\n> > > \t}\n> > > }\n> > >\n> > > -static const char *log_severity_name(LogSeverity severity)\n> > > +const char *log_severity_name(LogSeverity severity)\n> > > {\n> > > \tstatic const char *const names[] = {\n> > > \t\t\"DEBUG\",\n> > > @@ -99,6 +101,92 @@ static const char *log_severity_name(LogSeverity severity)\n> > > \t\treturn \"UNKWN\";\n> > > }\n> > >\n> > > +/**\n> > > + * \\brief Parse a log level string into a LogSeverity\n> > > + * \\param[in] level The log level string\n> > > + *\n> > > + * Log levels can be specified as an integer value in the range from LogDebug to\n> > > + * LogFatal, or as a string corresponding to the severity name in uppercase. Any\n> > > + * other value is invalid.\n> > > + *\n> > > + * \\return The log severity, or LogInvalid if the string is invalid\n> > > + */\n> > > +LogSeverity parseLogLevel(std::string_view level)\n> > > +{\n> > > +\tstatic const char *const names[] = {\n> > > +\t\t\"DEBUG\",\n> > > +\t\t\"INFO\",\n> > > +\t\t\"WARN\",\n> > > +\t\t\"ERROR\",\n> > > +\t\t\"FATAL\",\n> > > +\t};\n> > > +\n> > > +\tunsigned int severity;\n> > > +\n> > > +\tif (std::isdigit(level[0])) {\n> > > +\t\tauto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);\n> > > +\t\tif (ec != std::errc() || *end != '\\0' || severity > LogFatal)\n> > > +\t\t\tseverity = LogInvalid;\n> > > +\t} else {\n> > > +\t\tseverity = LogInvalid;\n> > > +\t\tfor (unsigned int i = 0; i < std::size(names); ++i) {\n> > > +\t\t\tif (names[i] == level) {\n> > > +\t\t\t\tseverity = i;\n> > > +\t\t\t\tbreak;\n> > > +\t\t\t}\n> > > +\t\t}\n> > > +\t}\n> > > +\n> > > +\treturn static_cast(severity);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Parse the log levels from the environment\n> > \n> > Missing \\param\n> \n> ACK\n> \n> > > + *\n> > > + * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable\n> > > + * as a list of \"category:level\" pairs, separated by commas (','). Parse the\n> > > + * variable and store the levels to configure all log categories.\n> > > + */\n> > > +void parseLogLevels(const char *line, std::list> &levels)\n> > > +{\n> > > +\tfor (const char *pair = line; *line != '\\0'; pair = line) {\n> > > +\t\tconst char *comma = strchrnul(line, ',');\n> > > +\t\tsize_t len = comma - pair;\n> > > +\n> > > +\t\t/* Skip over the comma. */\n> > > +\t\tline = *comma == ',' ? comma + 1 : comma;\n> > > +\n> > > +\t\t/* Skip to the next pair if the pair is empty. */\n> > > +\t\tif (!len)\n> > > +\t\t\tcontinue;\n> > > +\n> > > +\t\tstd::string_view category;\n> > > +\t\tstd::string_view level;\n> > > +\n> > > +\t\tconst char *colon = static_cast(memchr(pair, ':', len));\n> > > +\t\tif (!colon) {\n> > > +\t\t\t/* 'x' is a shortcut for '*:x'. */\n> > > +\t\t\tcategory = \"*\";\n> > > +\t\t\tlevel = std::string_view(pair, len);\n> > > +\t\t} else {\n> > > +\t\t\tcategory = std::string_view(pair, colon - pair);\n> > > +\t\t\tlevel = std::string_view(colon + 1, comma - colon - 1);\n> > > +\t\t}\n> > > +\n> > > +\t\t/* Both the category and the level must be specified. */\n> > > +\t\tif (category.empty() || level.empty())\n> > > +\t\t\tcontinue;\n> > > +\n> > > +\t\tLogSeverity severity = parseLogLevel(level);\n> > > +\t\tif (severity == LogInvalid)\n> > > +\t\t\tcontinue;\n> > > +\n> > > +\t\tlevels.emplace_back(category, severity);\n> > > +\t}\n> > > +}\n> > > +\n> > > +} /* namespace */\n> > > +\n> > > /**\n> > > * \\brief Log output\n> > > *\n> > > @@ -312,8 +400,6 @@ private:\n> > > \tLogger();\n> > >\n> > > \tvoid parseLogFile();\n> > > -\tvoid parseLogLevels();\n> > > -\tstatic LogSeverity parseLogLevel(std::string_view level);\n> > >\n> > > \tfriend LogCategory;\n> > > \tvoid registerCategory(LogCategory *category);\n> > > @@ -592,7 +678,9 @@ Logger::Logger()\n> > > \tlogSetStream(&std::cerr, color);\n> > >\n> > > \tparseLogFile();\n> > > -\tparseLogLevels();\n> > > +\n> > > +\tif (const char *debug = utils::secure_getenv(\"LIBCAMERA_LOG_LEVELS\"))\n> > > +\t\tparseLogLevels(debug, levels_);\n> > > }\n> > >\n> > > /**\n> > > @@ -618,94 +706,6 @@ void Logger::parseLogFile()\n> > > \tlogSetFile(file, false);\n> > > }\n> > >\n> > > -/**\n> > > - * \\brief Parse the log levels from the environment\n> > > - *\n> > > - * The log levels are stored in the LIBCAMERA_LOG_LEVELS environment variable\n> > > - * as a list of \"category:level\" pairs, separated by commas (','). Parse the\n> > > - * variable and store the levels to configure all log categories.\n> > > - */\n> > > -void Logger::parseLogLevels()\n> > > -{\n> > > -\tconst char *debug = utils::secure_getenv(\"LIBCAMERA_LOG_LEVELS\");\n> > > -\tif (!debug)\n> > > -\t\treturn;\n> > > -\n> > > -\tfor (const char *pair = debug; *debug != '\\0'; pair = debug) {\n> > > -\t\tconst char *comma = strchrnul(debug, ',');\n> > > -\t\tsize_t len = comma - pair;\n> > > -\n> > > -\t\t/* Skip over the comma. */\n> > > -\t\tdebug = *comma == ',' ? comma + 1 : comma;\n> > > -\n> > > -\t\t/* Skip to the next pair if the pair is empty. */\n> > > -\t\tif (!len)\n> > > -\t\t\tcontinue;\n> > > -\n> > > -\t\tstd::string_view category;\n> > > -\t\tstd::string_view level;\n> > > -\n> > > -\t\tconst char *colon = static_cast(memchr(pair, ':', len));\n> > > -\t\tif (!colon) {\n> > > -\t\t\t/* 'x' is a shortcut for '*:x'. */\n> > > -\t\t\tcategory = \"*\";\n> > > -\t\t\tlevel = std::string_view(pair, len);\n> > > -\t\t} else {\n> > > -\t\t\tcategory = std::string_view(pair, colon - pair);\n> > > -\t\t\tlevel = std::string_view(colon + 1, comma - colon - 1);\n> > > -\t\t}\n> > > -\n> > > -\t\t/* Both the category and the level must be specified. */\n> > > -\t\tif (category.empty() || level.empty())\n> > > -\t\t\tcontinue;\n> > > -\n> > > -\t\tLogSeverity severity = parseLogLevel(level);\n> > > -\t\tif (severity == LogInvalid)\n> > > -\t\t\tcontinue;\n> > > -\n> > > -\t\tlevels_.emplace_back(category, severity);\n> > > -\t}\n> > > -}\n> > > -\n> > > -/**\n> > > - * \\brief Parse a log level string into a LogSeverity\n> > > - * \\param[in] level The log level string\n> > > - *\n> > > - * Log levels can be specified as an integer value in the range from LogDebug to\n> > > - * LogFatal, or as a string corresponding to the severity name in uppercase. Any\n> > > - * other value is invalid.\n> > > - *\n> > > - * \\return The log severity, or LogInvalid if the string is invalid\n> > > - */\n> > > -LogSeverity Logger::parseLogLevel(std::string_view level)\n> > > -{\n> > > -\tstatic const char *const names[] = {\n> > > -\t\t\"DEBUG\",\n> > > -\t\t\"INFO\",\n> > > -\t\t\"WARN\",\n> > > -\t\t\"ERROR\",\n> > > -\t\t\"FATAL\",\n> > > -\t};\n> > > -\n> > > -\tunsigned int severity;\n> > > -\n> > > -\tif (std::isdigit(level[0])) {\n> > > -\t\tauto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);\n> > > -\t\tif (ec != std::errc() || *end != '\\0' || severity > LogFatal)\n> > > -\t\t\tseverity = LogInvalid;\n> > > -\t} else {\n> > > -\t\tseverity = LogInvalid;\n> > > -\t\tfor (unsigned int i = 0; i < std::size(names); ++i) {\n> > > -\t\t\tif (names[i] == level) {\n> > > -\t\t\t\tseverity = i;\n> > > -\t\t\t\tbreak;\n> > > -\t\t\t}\n> > > -\t\t}\n> > > -\t}\n> > > -\n> > > -\treturn static_cast(severity);\n> > > -}\n> > > -\n> > > /**\n> > > * \\brief Register a log category with the logger\n> > > * \\param[in] category The log category","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6E678C32EA\n\tfor ;\n\tThu, 6 Feb 2025 17:26:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2EFD3685F9;\n\tThu, 6 Feb 2025 18:26:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB6016053F\n\tfor ;\n\tThu, 6 Feb 2025 18:26:01 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 35BB01198;\n\tThu, 6 Feb 2025 18:24:48 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TvvPjiF0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1738862688;\n\tbh=wKMASRo6OwAzZCuqQXlzPSrZsmK9BCBPvA7WvDErrTA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TvvPjiF0cmgC5bYyIP3ZEbb3gOqfSIRKsj7iqZcdC6XiLMciEWRmrVsAhNVtKQksF\n\tosDNaLKKBDhu9OQ0kPH+tn/DVHLue/badVpS0yTWLcxanO7XroYBUsUB/tHIyKJMuh\n\t/hxPdA4LnBxwrrYi2ZBMpawg7eUdWoqPGc+PS9qQ=","Date":"Thu, 6 Feb 2025 19:25:57 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 7/9] libcamera: base: log: Split\n\t`parseLogLevel[s]()`","Message-ID":"<20250206172557.GD12400@pendragon.ideasonboard.com>","References":"<20250130195811.1230581-1-pobrn@protonmail.com>\n\t<20250130195811.1230581-8-pobrn@protonmail.com>\n\t<20250206164848.GA12400@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]