[{"id":33267,"web_url":"https://patchwork.libcamera.org/comment/33267/","msgid":"","date":"2025-02-03T17:13:41","subject":"Re: [RFC PATCH v2 8/9] libcamera: base: log: Protect log categories\n\twith lock","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Thu, Jan 30, 2025 at 07:58:57PM +0000, Barnabás Pőcze wrote:\n> Log categories may be added from any thread, so it is important\n> to synchronize access to the `Logger::categories_` list between\n> its two users: `Logger::findOrCreateCategory()` and `Logger::logSetLevel()`.\n>\n> To achieve that, `Logger::{find,register}Category()` are merged into\n> a single function: `Logger::findOrCreateCategory()`; and the mutex\n> from `LogCategory::create()` is moved into the `Logger` class.\n>\n> Furthermore, appropriate `MutexLocker`s are placed in\n> `Logger::findOrCreateCategory()` and `Logger::logSetLevel()`.\n>\n> Signed-off-by: Barnabás Pőcze \n\nThe two {find,register}Category() were indeed called consecutively, so\nfor the current usage is fine unifying the two..\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> include/libcamera/base/log.h | 2 +-\n> src/libcamera/base/log.cpp | 53 ++++++++++++------------------------\n> 2 files changed, 19 insertions(+), 36 deletions(-)\n>\n> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h\n> index 1fb92603f..b32ec4516 100644\n> --- a/include/libcamera/base/log.h\n> +++ b/include/libcamera/base/log.h\n> @@ -30,6 +30,7 @@ enum LogSeverity {\n> class LogCategory\n> {\n> public:\n> +\texplicit LogCategory(std::string_view name);\n> \tstatic LogCategory *create(std::string_view name);\n>\n> \tconst std::string &name() const { return name_; }\n> @@ -39,7 +40,6 @@ public:\n> \tstatic const LogCategory &defaultCategory();\n>\n> private:\n> -\texplicit LogCategory(std::string_view name);\n>\n> \tconst std::string name_;\n>\n> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> index 19fc5cc67..69aa16c4c 100644\n> --- a/src/libcamera/base/log.cpp\n> +++ b/src/libcamera/base/log.cpp\n> @@ -402,11 +402,11 @@ private:\n> \tvoid parseLogFile();\n>\n> \tfriend LogCategory;\n> -\tvoid registerCategory(LogCategory *category);\n> -\tLogCategory *findCategory(std::string_view name) const;\n> +\tLogCategory *findOrCreateCategory(std::string_view name);\n>\n> \tstatic bool destroyed_;\n>\n> +\tMutex mutex_;\n> \tstd::vector categories_;\n> \tstd::list> levels_;\n>\n> @@ -657,6 +657,8 @@ void Logger::logSetLevel(const char *category, const char *level)\n> \tif (severity == LogInvalid)\n> \t\treturn;\n>\n> +\tMutexLocker locker(mutex_);\n> +\n> \tfor (LogCategory *c : categories_) {\n> \t\tif (c->name() == category) {\n> \t\t\tc->setSeverity(severity);\n> @@ -707,17 +709,21 @@ void Logger::parseLogFile()\n> }\n>\n> /**\n> - * \\brief Register a log category with the logger\n> - * \\param[in] category The log category\n> - *\n> - * Log categories must have unique names. It is invalid to call this function\n> - * if a log category with the same name already exists.\n> + * \\brief Find an existing log category with the given name or create one\n> + * \\param[in] name Name of the log category\n> + * \\return The pointer to the log category found or created\n> */\n> -void Logger::registerCategory(LogCategory *category)\n> +LogCategory *Logger::findOrCreateCategory(std::string_view name)\n> {\n> -\tcategories_.push_back(category);\n> +\tMutexLocker locker(mutex_);\n> +\n> +\tfor (LogCategory *category : categories_) {\n> +\t\tif (category->name() == name)\n> +\t\t\treturn category;\n> +\t}\n> +\n> +\tLogCategory *category = categories_.emplace_back(new LogCategory(name));\n>\n> -\tconst std::string &name = category->name();\n> \tfor (const std::pair &level : levels_) {\n> \t\tbool match = true;\n>\n> @@ -737,22 +743,8 @@ void Logger::registerCategory(LogCategory *category)\n> \t\t\tbreak;\n> \t\t}\n> \t}\n> -}\n>\n> -/**\n> - * \\brief Find an existing log category with the given name\n> - * \\param[in] name Name of the log category\n> - * \\return The pointer to the found log category or nullptr if not found\n> - */\n> -LogCategory *Logger::findCategory(std::string_view name) const\n> -{\n> -\tif (auto it = std::find_if(categories_.begin(), categories_.end(),\n> -\t\t\t\t [name](auto c) { return c->name() == name; });\n> -\t it != categories_.end()) {\n> -\t\treturn *it;\n> -\t}\n> -\n> -\treturn nullptr;\n> +\treturn category;\n> }\n>\n> /**\n> @@ -790,16 +782,7 @@ LogCategory *Logger::findCategory(std::string_view name) const\n> */\n> LogCategory *LogCategory::create(std::string_view name)\n> {\n> -\tstatic Mutex mutex_;\n> -\tMutexLocker locker(mutex_);\n> -\tLogCategory *category = Logger::instance()->findCategory(name);\n> -\n> -\tif (!category) {\n> -\t\tcategory = new LogCategory(name);\n> -\t\tLogger::instance()->registerCategory(category);\n> -\t}\n> -\n> -\treturn category;\n> +\treturn Logger::instance()->findOrCreateCategory(name);\n> }\n>\n> /**\n> --\n> 2.48.1\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 88481C3260\n\tfor ;\n\tMon, 3 Feb 2025 17:13:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9973E6859A;\n\tMon, 3 Feb 2025 18:13:46 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CBB768592\n\tfor ;\n\tMon, 3 Feb 2025 18:13:44 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EC0EB664;\n\tMon, 3 Feb 2025 18:12:32 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"IAvGvQFC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1738602753;\n\tbh=4oEW0tbjQv+P7sdqcX+vLABod3eaZNPQ9BaS9XpEtHw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IAvGvQFCa1P2hIacUnzKr0no/Z6RymbAxlkASndtiNNS9v/8DLAnX7HrzOuIjflw/\n\tPrUQMirO44UlpGE7q9zVaRuHUE6VvN0CEXimAU8BC34dIN6dgh2eHcwe3ub5Y+wr6L\n\tvdD8DYwkTMm1JOVj+fh3j6AnVsUSDnUrXgQGfnfg=","Date":"Mon, 3 Feb 2025 18:13:41 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 8/9] libcamera: base: log: Protect log categories\n\twith lock","Message-ID":"","References":"<20250130195811.1230581-1-pobrn@protonmail.com>\n\t<20250130195811.1230581-9-pobrn@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250130195811.1230581-9-pobrn@protonmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33308,"web_url":"https://patchwork.libcamera.org/comment/33308/","msgid":"<20250206171118.GB12400@pendragon.ideasonboard.com>","date":"2025-02-06T17:11:18","subject":"Re: [RFC PATCH v2 8/9] libcamera: base: log: Protect log categories\n\twith lock","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the patch.\n\nOn Thu, Jan 30, 2025 at 07:58:57PM +0000, Barnabás Pőcze wrote:\n> Log categories may be added from any thread, so it is important\n> to synchronize access to the `Logger::categories_` list between\n> its two users: `Logger::findOrCreateCategory()` and `Logger::logSetLevel()`.\n\nfindOrCreateCategory() is a new function, it's not a current user. You\ncould write something along those lines:\n\nLog categories may be added from any thread, so it is important to\nsynchronize access to the `Logger::categories_` list between its two\nusers: category creation (by LogCategory::create(), which calls\nLogger::findCategory() and Logger::registerCategory()) and log level\nsetting (by Logger::logSetLevel()).\n\n> To achieve that, `Logger::{find,register}Category()` are merged into\n> a single function: `Logger::findOrCreateCategory()`; and the mutex\n> from `LogCategory::create()` is moved into the `Logger` class.\n> \n> Furthermore, appropriate `MutexLocker`s are placed in\n> `Logger::findOrCreateCategory()` and `Logger::logSetLevel()`.\n\nIt took me a bit of time to understand the patch, here's a proposal for\nan improved commit message.\n\nThe LogCategory::create() function uses a mutex to serialize category\ncreation, but Logger::logSetLevel() can access Logger::categories_\nconcurrently without any protection. To fix the issue, move the mutex to\nthe Logger class, and use it to protect all accesses to the categories\nlist. This requires moving all the logic of LogCategory::create() to a\nnew Logger::findOrCreateCategory() function that combines both\nLogger::findCategory() and Logger::registerCategory() in order to make\nthe two operations exacute atomically.\n\n> Signed-off-by: Barnabás Pőcze \n> ---\n> include/libcamera/base/log.h | 2 +-\n> src/libcamera/base/log.cpp | 53 ++++++++++++------------------------\n> 2 files changed, 19 insertions(+), 36 deletions(-)\n> \n> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h\n> index 1fb92603f..b32ec4516 100644\n> --- a/include/libcamera/base/log.h\n> +++ b/include/libcamera/base/log.h\n> @@ -30,6 +30,7 @@ enum LogSeverity {\n> class LogCategory\n> {\n> public:\n> +\texplicit LogCategory(std::string_view name);\n\nIt's not very nice to make this publicly available, as it shouldn't be\nused anywhere else than from the create() function (now) or the\nfindOrCreateCategory() function (with this patch applied). Wouldn't it\nbe better to keep the constructor private and make the Logger class a\nfriend ?\n\n> \tstatic LogCategory *create(std::string_view name);\n> \n> \tconst std::string &name() const { return name_; }\n> @@ -39,7 +40,6 @@ public:\n> \tstatic const LogCategory &defaultCategory();\n> \n> private:\n> -\texplicit LogCategory(std::string_view name);\n> \n> \tconst std::string name_;\n> \n> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> index 19fc5cc67..69aa16c4c 100644\n> --- a/src/libcamera/base/log.cpp\n> +++ b/src/libcamera/base/log.cpp\n> @@ -402,11 +402,11 @@ private:\n> \tvoid parseLogFile();\n> \n> \tfriend LogCategory;\n> -\tvoid registerCategory(LogCategory *category);\n> -\tLogCategory *findCategory(std::string_view name) const;\n> +\tLogCategory *findOrCreateCategory(std::string_view name);\n> \n> \tstatic bool destroyed_;\n> \n> +\tMutex mutex_;\n> \tstd::vector categories_;\n\nCan we add thread safety annotations to indicate that categories_ is\nprotected by mutex_ ?\n\n> \tstd::list> levels_;\n> \n> @@ -657,6 +657,8 @@ void Logger::logSetLevel(const char *category, const char *level)\n> \tif (severity == LogInvalid)\n> \t\treturn;\n> \n> +\tMutexLocker locker(mutex_);\n> +\n> \tfor (LogCategory *c : categories_) {\n> \t\tif (c->name() == category) {\n> \t\t\tc->setSeverity(severity);\n> @@ -707,17 +709,21 @@ void Logger::parseLogFile()\n> }\n> \n> /**\n> - * \\brief Register a log category with the logger\n> - * \\param[in] category The log category\n> - *\n> - * Log categories must have unique names. It is invalid to call this function\n> - * if a log category with the same name already exists.\n> + * \\brief Find an existing log category with the given name or create one\n> + * \\param[in] name Name of the log category\n> + * \\return The pointer to the log category found or created\n> */\n> -void Logger::registerCategory(LogCategory *category)\n> +LogCategory *Logger::findOrCreateCategory(std::string_view name)\n> {\n> -\tcategories_.push_back(category);\n> +\tMutexLocker locker(mutex_);\n> +\n> +\tfor (LogCategory *category : categories_) {\n> +\t\tif (category->name() == name)\n> +\t\t\treturn category;\n> +\t}\n\nSo nobody likes std::find_if() ? :-( I can survive its removal.\n\n> +\n> +\tLogCategory *category = categories_.emplace_back(new LogCategory(name));\n> \n> -\tconst std::string &name = category->name();\n> \tfor (const std::pair &level : levels_) {\n> \t\tbool match = true;\n> \n> @@ -737,22 +743,8 @@ void Logger::registerCategory(LogCategory *category)\n> \t\t\tbreak;\n> \t\t}\n> \t}\n> -}\n> \n> -/**\n> - * \\brief Find an existing log category with the given name\n> - * \\param[in] name Name of the log category\n> - * \\return The pointer to the found log category or nullptr if not found\n> - */\n> -LogCategory *Logger::findCategory(std::string_view name) const\n> -{\n> -\tif (auto it = std::find_if(categories_.begin(), categories_.end(),\n> -\t\t\t\t [name](auto c) { return c->name() == name; });\n> -\t it != categories_.end()) {\n> -\t\treturn *it;\n> -\t}\n> -\n> -\treturn nullptr;\n> +\treturn category;\n> }\n> \n> /**\n> @@ -790,16 +782,7 @@ LogCategory *Logger::findCategory(std::string_view name) const\n> */\n> LogCategory *LogCategory::create(std::string_view name)\n> {\n> -\tstatic Mutex mutex_;\n> -\tMutexLocker locker(mutex_);\n> -\tLogCategory *category = Logger::instance()->findCategory(name);\n> -\n> -\tif (!category) {\n> -\t\tcategory = new LogCategory(name);\n> -\t\tLogger::instance()->registerCategory(category);\n> -\t}\n> -\n> -\treturn category;\n> +\treturn Logger::instance()->findOrCreateCategory(name);\n> }\n> \n> /**","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 98357C32EA\n\tfor ;\n\tThu, 6 Feb 2025 17:11:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AECF2685EE;\n\tThu, 6 Feb 2025 18:11:25 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CC2C6053F\n\tfor ;\n\tThu, 6 Feb 2025 18:11:23 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AD4B71198;\n\tThu, 6 Feb 2025 18:10:09 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"c6gve/HL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1738861809;\n\tbh=mK74Dyet6nDr5b6GGjnFw/YTNmfXFSLbhrpOMZfIVK0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=c6gve/HLvKtIq5ggDEDYl8rwsESagxf6Otg9etqrE+bEoJ9TCJtI7cLLFiEH2vCXG\n\tniW93UlEpCFtKd7zybuQXg0RnLHCDNfqHI480QItWikPAzEbQgkuhwAQXDs4dvcqU5\n\tMLLp+TiImEeD2Ksfa9tz1BfsomwAcyzyXNnCvm6s=","Date":"Thu, 6 Feb 2025 19:11:18 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 8/9] libcamera: base: log: Protect log categories\n\twith lock","Message-ID":"<20250206171118.GB12400@pendragon.ideasonboard.com>","References":"<20250130195811.1230581-1-pobrn@protonmail.com>\n\t<20250130195811.1230581-9-pobrn@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250130195811.1230581-9-pobrn@protonmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33317,"web_url":"https://patchwork.libcamera.org/comment/33317/","msgid":"<6Udh_elDZ_L16jKnV08JvaJgrRL8CPdIft1B_yWXqHDgnSAZIdqloly2ugrX1VdAqTshykVMzTABgQMKdoR5zfSbNtSIIyHc6I1lFHRrZKU=@protonmail.com>","date":"2025-02-06T17:46:44","subject":"Re: [RFC PATCH v2 8/9] libcamera: base: log: Protect log categories\n\twith lock","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2025. február 6., csütörtök 18:11 keltezéssel, Laurent Pinchart írta:\n\n> Hi Barnabás,\n> \n> Thank you for the patch.\n> \n> On Thu, Jan 30, 2025 at 07:58:57PM +0000, Barnabás Pőcze wrote:\n> > Log categories may be added from any thread, so it is important\n> > to synchronize access to the `Logger::categories_` list between\n> > its two users: `Logger::findOrCreateCategory()` and `Logger::logSetLevel()`.\n> \n> findOrCreateCategory() is a new function, it's not a current user. You\n> could write something along those lines:\n> \n> Log categories may be added from any thread, so it is important to\n> synchronize access to the `Logger::categories_` list between its two\n> users: category creation (by LogCategory::create(), which calls\n> Logger::findCategory() and Logger::registerCategory()) and log level\n> setting (by Logger::logSetLevel()).\n> \n> > To achieve that, `Logger::{find,register}Category()` are merged into\n> > a single function: `Logger::findOrCreateCategory()`; and the mutex\n> > from `LogCategory::create()` is moved into the `Logger` class.\n> >\n> > Furthermore, appropriate `MutexLocker`s are placed in\n> > `Logger::findOrCreateCategory()` and `Logger::logSetLevel()`.\n> \n> It took me a bit of time to understand the patch, here's a proposal for\n> an improved commit message.\n> \n> The LogCategory::create() function uses a mutex to serialize category\n> creation, but Logger::logSetLevel() can access Logger::categories_\n> concurrently without any protection. To fix the issue, move the mutex to\n> the Logger class, and use it to protect all accesses to the categories\n> list. This requires moving all the logic of LogCategory::create() to a\n> new Logger::findOrCreateCategory() function that combines both\n> Logger::findCategory() and Logger::registerCategory() in order to make\n> the two operations exacute atomically.\n\nACK\n\n\n> \n> > Signed-off-by: Barnabás Pőcze \n> > ---\n> > include/libcamera/base/log.h | 2 +-\n> > src/libcamera/base/log.cpp | 53 ++++++++++++------------------------\n> > 2 files changed, 19 insertions(+), 36 deletions(-)\n> >\n> > diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h\n> > index 1fb92603f..b32ec4516 100644\n> > --- a/include/libcamera/base/log.h\n> > +++ b/include/libcamera/base/log.h\n> > @@ -30,6 +30,7 @@ enum LogSeverity {\n> > class LogCategory\n> > {\n> > public:\n> > +\texplicit LogCategory(std::string_view name);\n> \n> It's not very nice to make this publicly available, as it shouldn't be\n> used anywhere else than from the create() function (now) or the\n> findOrCreateCategory() function (with this patch applied). Wouldn't it\n> be better to keep the constructor private and make the Logger class a\n> friend ?\n\nI think this change might be here by mistake. I think it might only be needed\nfor the last patch. (It is unfortunate that having only private constructors\nprevents the use of `make_{unique,shared}()`, `emplace_back()`, etc.)\n\n\n> \n> > \tstatic LogCategory *create(std::string_view name);\n> >\n> > \tconst std::string &name() const { return name_; }\n> > @@ -39,7 +40,6 @@ public:\n> > \tstatic const LogCategory &defaultCategory();\n> >\n> > private:\n> > -\texplicit LogCategory(std::string_view name);\n> >\n> > \tconst std::string name_;\n> >\n> > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> > index 19fc5cc67..69aa16c4c 100644\n> > --- a/src/libcamera/base/log.cpp\n> > +++ b/src/libcamera/base/log.cpp\n> > @@ -402,11 +402,11 @@ private:\n> > \tvoid parseLogFile();\n> >\n> > \tfriend LogCategory;\n> > -\tvoid registerCategory(LogCategory *category);\n> > -\tLogCategory *findCategory(std::string_view name) const;\n> > +\tLogCategory *findOrCreateCategory(std::string_view name);\n> >\n> > \tstatic bool destroyed_;\n> >\n> > +\tMutex mutex_;\n> > \tstd::vector categories_;\n> \n> Can we add thread safety annotations to indicate that categories_ is\n> protected by mutex_ ?\n\nI think so.\n\n\n> \n> > \tstd::list> levels_;\n> >\n> > @@ -657,6 +657,8 @@ void Logger::logSetLevel(const char *category, const char *level)\n> > \tif (severity == LogInvalid)\n> > \t\treturn;\n> >\n> > +\tMutexLocker locker(mutex_);\n> > +\n> > \tfor (LogCategory *c : categories_) {\n> > \t\tif (c->name() == category) {\n> > \t\t\tc->setSeverity(severity);\n> > @@ -707,17 +709,21 @@ void Logger::parseLogFile()\n> > }\n> >\n> > /**\n> > - * \\brief Register a log category with the logger\n> > - * \\param[in] category The log category\n> > - *\n> > - * Log categories must have unique names. It is invalid to call this function\n> > - * if a log category with the same name already exists.\n> > + * \\brief Find an existing log category with the given name or create one\n> > + * \\param[in] name Name of the log category\n> > + * \\return The pointer to the log category found or created\n> > */\n> > -void Logger::registerCategory(LogCategory *category)\n> > +LogCategory *Logger::findOrCreateCategory(std::string_view name)\n> > {\n> > -\tcategories_.push_back(category);\n> > +\tMutexLocker locker(mutex_);\n> > +\n> > +\tfor (LogCategory *category : categories_) {\n> > +\t\tif (category->name() == name)\n> > +\t\t\treturn category;\n> > +\t}\n> \n> So nobody likes std::find_if() ? :-( I can survive its removal.\n> \n> > +\n> > +\tLogCategory *category = categories_.emplace_back(new LogCategory(name));\n> >\n> > -\tconst std::string &name = category->name();\n> > \tfor (const std::pair &level : levels_) {\n> > \t\tbool match = true;\n> >\n> > @@ -737,22 +743,8 @@ void Logger::registerCategory(LogCategory *category)\n> > \t\t\tbreak;\n> > \t\t}\n> > \t}\n> > -}\n> >\n> > -/**\n> > - * \\brief Find an existing log category with the given name\n> > - * \\param[in] name Name of the log category\n> > - * \\return The pointer to the found log category or nullptr if not found\n> > - */\n> > -LogCategory *Logger::findCategory(std::string_view name) const\n> > -{\n> > -\tif (auto it = std::find_if(categories_.begin(), categories_.end(),\n> > -\t\t\t\t [name](auto c) { return c->name() == name; });\n> > -\t it != categories_.end()) {\n> > -\t\treturn *it;\n> > -\t}\n> > -\n> > -\treturn nullptr;\n> > +\treturn category;\n> > }\n> >\n> > /**\n> > @@ -790,16 +782,7 @@ LogCategory *Logger::findCategory(std::string_view name) const\n> > */\n> > LogCategory *LogCategory::create(std::string_view name)\n> > {\n> > -\tstatic Mutex mutex_;\n> > -\tMutexLocker locker(mutex_);\n> > -\tLogCategory *category = Logger::instance()->findCategory(name);\n> > -\n> > -\tif (!category) {\n> > -\t\tcategory = new LogCategory(name);\n> > -\t\tLogger::instance()->registerCategory(category);\n> > -\t}\n> > -\n> > -\treturn category;\n> > +\treturn Logger::instance()->findOrCreateCategory(name);\n> > }\n> >\n> > /**\n> \n> --\n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 33A7EC32EF\n\tfor ;\n\tThu, 6 Feb 2025 17:46:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5929F68600;\n\tThu, 6 Feb 2025 18:46:50 +0100 (CET)","from mail-40133.protonmail.ch (mail-40133.protonmail.ch\n\t[185.70.40.133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 56D8C685F6\n\tfor ;\n\tThu, 6 Feb 2025 18:46:48 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"ciGe0gip\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1738864007; x=1739123207;\n\tbh=S+m6W4/tukwclaoGlDDYvBGfxTip6GkADhmTscoGzxA=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;\n\tb=ciGe0gipEhn9BDLbYPTJBlhNr/rsh2oPnECpuBDmmknwfuPcOoXuKUB81VzZBfgbh\n\tai6okkziD7pFqzMfSdCXGvkou/k1aYq2m4BuZAeNIcKWPUHIZOVU0SWN8a0g5Qggnc\n\tx4ixx3X03cjvHXy8GE02XyVKYloZd239rp4H6RNS9k4ewvHTCgIlTzYIurC3feRFsp\n\t1f9zyhQgIzuqWx23GItkgMf7eH18rVk6w+vees9dBCYBN1RMyVLjjovMl3nbvMvzsq\n\tX+Ttfq7Xma3geAtJqjT6Cyd0WS7JS8+uIP4ARx0UWkqk9O/LUnSboQA/eQo8ItiFhr\n\tpBPGdsfIerMIA==","Date":"Thu, 06 Feb 2025 17:46:44 +0000","To":"Laurent Pinchart ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 8/9] libcamera: base: log: Protect log categories\n\twith lock","Message-ID":"<6Udh_elDZ_L16jKnV08JvaJgrRL8CPdIft1B_yWXqHDgnSAZIdqloly2ugrX1VdAqTshykVMzTABgQMKdoR5zfSbNtSIIyHc6I1lFHRrZKU=@protonmail.com>","In-Reply-To":"<20250206171118.GB12400@pendragon.ideasonboard.com>","References":"<20250130195811.1230581-1-pobrn@protonmail.com>\n\t<20250130195811.1230581-9-pobrn@protonmail.com>\n\t<20250206171118.GB12400@pendragon.ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"ea93754c8c7870cc2bebfd7f35fa8cafdf4c14a0","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]