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

[Kieran Bingham]

Quoting Laurent Pinchart (2025-01-22 12:24:12)
> 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>

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

> 
> >  }
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart