libcamera: base: log: Fix LogCategory creation issues

Each declaration of a LogCategory will create a new LogCategory, and
will be stored in an unordered_set Logger::categories_. This means that
when a plugin .so is unloaded and loaded, as happens when destructing
and creating a CamereManager, we'll get duplicate categories.

The Logger::registerCategory docs say "Log categories must have unique
names. If a category with the same name already exists this function
performs no operation.". The code does not comply with this.

We solve the issue with two changes:

Change the unordered_set to a vector for simplicity, as there's no need
for an unordered_set.

Instead of using the LogCategory constructor to create new categories in
_LOG_CATEGORY() macro, use a factory method. The factory method will
return either an existing LogCategory if one exists with the given name,
or a newly created one.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
This commit is contained in:
Tomi Valkeinen 2022-08-29 11:44:45 +03:00 committed by Laurent Pinchart
parent 662df4ca26
commit ab2a10f007
2 changed files with 47 additions and 8 deletions

View file

@ -29,7 +29,7 @@ enum LogSeverity {
class LogCategory class LogCategory
{ {
public: public:
explicit LogCategory(const char *name); static LogCategory *create(const char *name);
const std::string &name() const { return name_; } const std::string &name() const { return name_; }
LogSeverity severity() const { return severity_; } LogSeverity severity() const { return severity_; }
@ -38,6 +38,8 @@ public:
static const LogCategory &defaultCategory(); static const LogCategory &defaultCategory();
private: private:
explicit LogCategory(const char *name);
const std::string name_; const std::string name_;
LogSeverity severity_; LogSeverity severity_;
}; };
@ -49,7 +51,7 @@ extern const LogCategory &_LOG_CATEGORY(name)();
const LogCategory &_LOG_CATEGORY(name)() \ const LogCategory &_LOG_CATEGORY(name)() \
{ \ { \
/* The instance will be deleted by the Logger destructor. */ \ /* The instance will be deleted by the Logger destructor. */ \
static LogCategory *category = new LogCategory(#name); \ static LogCategory *category = LogCategory::create(#name); \
return *category; \ return *category; \
} }

View file

@ -314,10 +314,11 @@ private:
friend LogCategory; friend LogCategory;
void registerCategory(LogCategory *category); void registerCategory(LogCategory *category);
LogCategory *findCategory(const char *name) const;
static bool destroyed_; static bool destroyed_;
std::unordered_set<LogCategory *> categories_; std::vector<LogCategory *> categories_;
std::list<std::pair<std::string, LogSeverity>> levels_; std::list<std::pair<std::string, LogSeverity>> levels_;
std::shared_ptr<LogOutput> output_; std::shared_ptr<LogOutput> output_;
@ -707,12 +708,12 @@ LogSeverity Logger::parseLogLevel(const std::string &level)
* \brief Register a log category with the logger * \brief Register a log category with the logger
* \param[in] category The log category * \param[in] category The log category
* *
* Log categories must have unique names. If a category with the same name * Log categories must have unique names. It is invalid to call this function
* already exists this function performs no operation. * if a log category with the same name already exists.
*/ */
void Logger::registerCategory(LogCategory *category) void Logger::registerCategory(LogCategory *category)
{ {
categories_.insert(category); categories_.push_back(category);
const std::string &name = category->name(); const std::string &name = category->name();
for (const std::pair<std::string, LogSeverity> &level : levels_) { for (const std::pair<std::string, LogSeverity> &level : levels_) {
@ -736,6 +737,22 @@ void Logger::registerCategory(LogCategory *category)
} }
} }
/**
* \brief Find an existing log category with the given name
* \param[in] name Name of the log category
* \return The pointer to the found log category or nullptr if not found
*/
LogCategory *Logger::findCategory(const char *name) const
{
if (auto it = std::find_if(categories_.begin(), categories_.end(),
[name](auto c) { return c->name() == name; });
it != categories_.end()) {
return *it;
}
return nullptr;
}
/** /**
* \enum LogSeverity * \enum LogSeverity
* Log message severity * Log message severity
@ -760,6 +777,27 @@ void Logger::registerCategory(LogCategory *category)
* and is used to control the log level per group. * and is used to control the log level per group.
*/ */
/**
* \brief Create a new LogCategory or return an existing one
* \param[in] name Name of the log category
*
* Create and return a new LogCategory with the given name if such a category
* does not yet exist, or return the existing one.
*
* \return The pointer to the LogCategory
*/
LogCategory *LogCategory::create(const char *name)
{
LogCategory *category = Logger::instance()->findCategory(name);
if (!category) {
category = new LogCategory(name);
Logger::instance()->registerCategory(category);
}
return category;
}
/** /**
* \brief Construct a log category * \brief Construct a log category
* \param[in] name The category name * \param[in] name The category name
@ -767,7 +805,6 @@ void Logger::registerCategory(LogCategory *category)
LogCategory::LogCategory(const char *name) LogCategory::LogCategory(const char *name)
: name_(name), severity_(LogSeverity::LogInfo) : name_(name), severity_(LogSeverity::LogInfo)
{ {
Logger::instance()->registerCategory(this);
} }
/** /**
@ -804,7 +841,7 @@ void LogCategory::setSeverity(LogSeverity severity)
*/ */
const LogCategory &LogCategory::defaultCategory() const LogCategory &LogCategory::defaultCategory()
{ {
static const LogCategory *category = new LogCategory("default"); static const LogCategory *category = LogCategory::create("default");
return *category; return *category;
} }