Patch Detail
Show a patch.
GET /api/1.1/patches/22877/?format=api
{ "id": 22877, "url": "https://patchwork.libcamera.org/api/1.1/patches/22877/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22877/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20250225173531.2595922-8-pobrn@protonmail.com>", "date": "2025-02-25T17:36:16", "name": "[v4,7/8] libcamera: base: log: Protect log categories with lock", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "c19d94086654c9b2ebee7583112387dbd726bdbf", "submitter": { "id": 133, "url": "https://patchwork.libcamera.org/api/1.1/people/133/?format=api", "name": "Pőcze Barnabás", "email": "pobrn@protonmail.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22877/mbox/", "series": [ { "id": 5022, "url": "https://patchwork.libcamera.org/api/1.1/series/5022/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5022", "date": "2025-02-25T17:35:34", "name": "libcamera: base: log: Misc. changes", "version": 4, "mbox": "https://patchwork.libcamera.org/series/5022/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22877/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22877/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 4C65DBDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 25 Feb 2025 17:36:25 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9F4BD6873C;\n\tTue, 25 Feb 2025 18:36:24 +0100 (CET)", "from mail-4316.protonmail.ch (mail-4316.protonmail.ch\n\t[185.70.43.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BBBAB686D6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 25 Feb 2025 18:36:22 +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=\"vlD/o1pp\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1740504982; x=1740764182;\n\tbh=3o/9e0VmUVhmnmrr6PgXseKVF982IpuzpXNd86t5QDw=;\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=vlD/o1pp+McEMLxQ5WuIl1otJLFrCBbrAArAY0s5NY3FXJs6PB/cyyqFEHdsI9IsS\n\t2WlCwHRGe9DO/Rnmw4Nkvtu4uyKyXG9gPqQ1DxXvTmPysRMaeY052j3EIOMeXkEits\n\tPoFi9BPHWfvcHH2vODDozDA/FhbPjk6Yfair40qVntD0fwvCru6Ld//83UP+O6j+em\n\t6f1YibYN51vyqfXLLUuckAcaUPkdK+76ZLytmQSyI/CJ8Ub1ST14EjAtL/zgdsjvuN\n\tsWCL2IKehZJjYOMGNYEZbxr6Tla4RtopYz0nPv6WV1g4jR43M1iQ3gDtywY7MxBF3+\n\tSMLefFjV0ntHw==", "Date": "Tue, 25 Feb 2025 17:36:16 +0000", "To": "libcamera-devel@lists.libcamera.org", "From": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>", "Cc": "Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>", "Subject": "[PATCH v4 7/8] libcamera: base: log: Protect log categories with\n\tlock", "Message-ID": "<20250225173531.2595922-8-pobrn@protonmail.com>", "In-Reply-To": "<20250225173531.2595922-1-pobrn@protonmail.com>", "References": "<20250225173531.2595922-1-pobrn@protonmail.com>", "Feedback-ID": "20568564:user:proton", "X-Pm-Message-ID": "6e7f7f3c8d198df5efde20fe48d732c3cb9c68aa", "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": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Log 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\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\nSigned-off-by: Barnabás Pőcze <pobrn@protonmail.com>\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/base/log.h | 1 +\n src/libcamera/base/log.cpp | 58 +++++++++++++-----------------------\n 2 files changed, 22 insertions(+), 37 deletions(-)", "diff": "diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h\nindex 1fb92603f..8af74b59d 100644\n--- a/include/libcamera/base/log.h\n+++ b/include/libcamera/base/log.h\n@@ -39,6 +39,7 @@ public:\n \tstatic const LogCategory &defaultCategory();\n \n private:\n+\tfriend class Logger;\n \texplicit LogCategory(std::string_view name);\n \n \tconst std::string name_;\ndiff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\nindex 6687deb93..155355f0c 100644\n--- a/src/libcamera/base/log.cpp\n+++ b/src/libcamera/base/log.cpp\n@@ -317,12 +317,12 @@ private:\n \tstatic LogSeverity parseLogLevel(std::string_view level);\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-\tstd::vector<LogCategory *> categories_;\n+\tMutex mutex_;\n+\tstd::vector<LogCategory *> categories_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n \tstd::list<std::pair<std::string, LogSeverity>> levels_;\n \n \tstd::shared_ptr<LogOutput> output_;\n@@ -572,6 +572,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@@ -708,37 +710,28 @@ LogSeverity Logger::parseLogLevel(std::string_view level)\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-\tconst std::string &name = category->name();\n-\tfor (const auto &[pattern, severity] : levels_) {\n-\t\tif (fnmatch(pattern.c_str(), name.c_str(), FNM_NOESCAPE) == 0)\n-\t\t\tcategory->setSeverity(severity);\n+\tfor (LogCategory *category : categories_) {\n+\t\tif (category->name() == name)\n+\t\t\treturn category;\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+\tLogCategory *category = categories_.emplace_back(new LogCategory(name));\n+\tconst char *categoryName = category->name().c_str();\n+\n+\tfor (const auto &[pattern, severity] : levels_) {\n+\t\tif (fnmatch(pattern.c_str(), categoryName, FNM_NOESCAPE) == 0)\n+\t\t\tcategory->setSeverity(severity);\n \t}\n \n-\treturn nullptr;\n+\treturn category;\n }\n \n /**\n@@ -776,16 +769,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", "prefixes": [ "v4", "7/8" ] }