[v3] libcamera: log: Match whole category in LIBCAMERA_LOG_LEVELS
diff mbox series

Message ID 20250123090421.33226-1-stefan.klug@ideasonboard.com
State Accepted
Commit 9b1f609e5b754ce7adc0219a08b72714ae783e43
Headers show
Series
  • [v3] libcamera: log: Match whole category in LIBCAMERA_LOG_LEVELS
Related show

Commit Message

Stefan Klug Jan. 23, 2025, 9:04 a.m. UTC
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@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---

Changes in v2:
- Replaced matching code with fnmatch instead of a manual fix

Changes in v3:
- Collected tag
- Updated documentation to no longer restrict the wildcard to the end
  of the string
---
 src/libcamera/base/log.cpp | 26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

Comments

Kieran Bingham Feb. 6, 2025, 12:09 p.m. UTC | #1
Quoting Stefan Klug (2025-01-23 09:04:18)
> 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@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 

Or even this one too.

If only there was a way to keep a specific work topic all bundled
together in a centralised way. :-(

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
> 
> Changes in v2:
> - Replaced matching code with fnmatch instead of a manual fix
> 
> Changes in v3:
> - Collected tag
> - Updated documentation to no longer restrict the wildcard to the end
>   of the string
> ---
>  src/libcamera/base/log.cpp | 26 ++++++--------------------
>  1 file changed, 6 insertions(+), 20 deletions(-)
> 
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 3a656b8f099f..72e0db859943 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>
> @@ -38,8 +39,8 @@
>   * The levels are configurable through the LIBCAMERA_LOG_LEVELS environment
>   * variable that contains a comma-separated list of 'category:level' pairs.
>   *
> - * The category names are strings and can include a wildcard ('*') character at
> - * the end to match multiple categories.
> + * The category names are strings and can include a wildcard ('*') character to
> + * match multiple categories.
>   *
>   * The level are either numeric values, or strings containing the log level
>   * name. The available log levels are DEBUG, INFO, WARN, ERROR and FATAL. Log
> @@ -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);
>         }
>  }
>  
> -- 
> 2.43.0
>

Patch
diff mbox series

diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
index 3a656b8f099f..72e0db859943 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>
@@ -38,8 +39,8 @@ 
  * The levels are configurable through the LIBCAMERA_LOG_LEVELS environment
  * variable that contains a comma-separated list of 'category:level' pairs.
  *
- * The category names are strings and can include a wildcard ('*') character at
- * the end to match multiple categories.
+ * The category names are strings and can include a wildcard ('*') character to
+ * match multiple categories.
  *
  * The level are either numeric values, or strings containing the log level
  * name. The available log levels are DEBUG, INFO, WARN, ERROR and FATAL. Log
@@ -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);
 	}
 }