[PATCH] libcamera: base: Fix log level parsing when multiple categories are listed

[Barnabás Pőcze]

2025. 06. 06. 10:37 keltezéssel, Stefan Klug írta:
> For a list of log levels like LIBCAMERA_LOG_LEVELS="CatA:0,CatB:1" only
> the severity of the last entry is correctly parsed.
> 
> Due to the change of level to a string_view in 24c2caa1c1b3 ("libcamera:
> base: log: Use `std::string_view` to avoid some copies") the level is no
> longer necessarily null terminated as it is a view on the original data.
> 
> Replace the check for a terminating null by a check for the end position
> to fix the issue.

Oops, sorry. I hope it wasn't too hard to find.


> 
> Fixes: 24c2caa1c1b3 ("libcamera: base: log: Use `std::string_view` to avoid some copies")
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>   src/libcamera/base/log.cpp | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 8bf3e1daa9c6..e024b58d7180 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -690,8 +690,9 @@ LogSeverity Logger::parseLogLevel(std::string_view level)
>   	unsigned int severity = LogInvalid;
>   
>   	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)
> +		const char* levelEnd = level.data() + level.size();

const char *levelEnd ?

Could a test be added? Maybe something like this:

---
diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp
index 0b999738d..8587ea30d 100644
--- a/test/log/log_api.cpp
+++ b/test/log/log_api.cpp
@@ -26,10 +26,42 @@ using namespace std;
  using namespace libcamera;
  
  LOG_DEFINE_CATEGORY(LogAPITest)
+LOG_DEFINE_CATEGORY(Cat0)
+LOG_DEFINE_CATEGORY(Cat1)
+LOG_DEFINE_CATEGORY(Cat2)
+LOG_DEFINE_CATEGORY(Cat3)
+LOG_DEFINE_CATEGORY(Cat4)
  
  class LogAPITest : public Test
  {
  protected:
+	int testEnvLevels()
+	{
+		setenv("LIBCAMERA_LOG_LEVELS",
+		       "Cat0:0,Cat0:9999,Cat1:INFO,Cat1:INVALID,Cat2:2,Cat2:-1,Cat3:ERROR,Cat3:{[]},Cat4:4,Cat4:rubbish",
+		       true);
+		logSetTarget(libcamera::LoggingTargetNone);
+
+		const std::pair<const LogCategory &, libcamera::LogSeverity> expected[] = {
+			{ _LOG_CATEGORY(Cat0)(), libcamera::LogDebug },
+			{ _LOG_CATEGORY(Cat1)(), libcamera::LogInfo },
+			{ _LOG_CATEGORY(Cat2)(), libcamera::LogWarning },
+			{ _LOG_CATEGORY(Cat3)(), libcamera::LogError },
+			{ _LOG_CATEGORY(Cat4)(), libcamera::LogFatal },
+		};
+		bool ok = true;
+
+		for (const auto &[c, s] : expected) {
+			if (c.severity() != s) {
+				ok = false;
+				cerr << "Severity of " << c.name() << " (" << c.severity() << ") "
+				     << "does not equal " << s << endl;
+			}
+		}
+
+		return ok ? TestPass : TestFail;
+	}
+
  	void doLogging()
  	{
  		logSetLevel("LogAPITest", "DEBUG");
@@ -135,7 +167,11 @@ protected:
  
  	int run() override
  	{
-		int ret = testFile();
+		int ret = testEnvLevels();
+		if (ret != TestPass)
+			return TestFail;
+
+		ret = testFile();
  		if (ret != TestPass)
  			return TestFail;
  
---

Reviewed-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>


> +		auto [end, ec] = std::from_chars(level.data(), levelEnd, severity);
> +		if (ec != std::errc() || end != levelEnd || severity > LogFatal)
>   			severity = LogInvalid;
>   	} else {
>   		for (unsigned int i = 0; i < std::size(names); ++i) {