[PATCH v2] libcamera: log: Match whole category in LIBCAMERA_LOG_LEVELS

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Wed, Jan 22, 2025 at 11:12:19AM +0100, Stefan Klug wrote:
> A LIBCAMERA_LOG_LEVELS value of "RkISP1:0" also applies to RkISP1Ccm and
> RkISP1Awb. This behavior is unexpected as it automatically enables all
> algorithm log categories when the intent is only to increase the log
> level of the upper category. Fix that replacing the manual matching code
> with fnmatch. This has the side effect that more wildcards ("?" and
> "[...]") are supported which is acceptable but won't be advertised.
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> 
> ---
> 
> Changes in v2:
> - Replaced matching code with fnmatch instead of a manual fix
> ---
>  src/libcamera/base/log.cpp | 22 ++++------------------
>  1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 3a656b8f099f..086103b328e6 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -8,6 +8,7 @@
>  #include <libcamera/base/log.h>
>  
>  #include <array>
> +#include <fnmatch.h>
>  #include <fstream>
>  #include <iostream>
>  #include <list>
> @@ -717,24 +718,9 @@ void Logger::registerCategory(LogCategory *category)
>  	categories_.push_back(category);
>  
>  	const std::string &name = category->name();
> -	for (const std::pair<std::string, LogSeverity> &level : levels_) {
> -		bool match = true;
> -
> -		for (unsigned int i = 0; i < level.first.size(); ++i) {
> -			if (level.first[i] == '*')
> -				break;
> -
> -			if (i >= name.size() ||
> -			    name[i] != level.first[i]) {
> -				match = false;
> -				break;
> -			}
> -		}
> -
> -		if (match) {
> -			category->setSeverity(level.second);
> -			break;
> -		}
> +	for (const auto &[pattern, severity] : levels_) {
> +		if (fnmatch(pattern.c_str(), name.c_str(), FNM_NOESCAPE) == 0)
> +			category->setSeverity(severity);
>  	}

You also need to update the comment at the beginning of the file to
indicate the wildcard can be located anywhere. With that,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  }
>  

-- 
Regards,

Laurent Pinchart