From patchwork Mon Feb 17 18:54:40 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22801 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id C4F08C3200 for ; Mon, 17 Feb 2025 18:54:47 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7C4C268686; Mon, 17 Feb 2025 19:54:47 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="as1axuY4"; dkim-atps=neutral Received: from mail-10628.protonmail.ch (mail-10628.protonmail.ch [79.135.106.28]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9767A6867F for ; Mon, 17 Feb 2025 19:54:46 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1739818485; x=1740077685; bh=A+W+78pmn5VywPajj1M639brMT/ZaiOPzqKEXLum3aI=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=as1axuY4+QaqbvKgLP7fLVl+GyG7Etd4D3RhpTTVHxdUC9P1JHx+dVgExSymxFje9 EGYF0krqRt8/61Walm7WI2Px8nM04hrfbjevkbBODFKc0ZDbaUsy04z6e0aZQIOZ7H wZTwooSz85vdsYBNDj21Bn7HCpFWdFQOGmczMoJp5l5Ci2R1oWsPeKIAkUqtwjk5xa /vVIivjfp8qMRJsIzSmIF6zjix9ytBWUOm6QnqSfajI/Btw8WoipnF8ZfLaEMvvoLW vSD9OLn3H2QnuZoPONa5q64dqYX5CQSykvISTuyVUel9x3OOCPUWQqFXMatd6g/QVG OKnd97g4VsDJw== Date: Mon, 17 Feb 2025 18:54:40 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Cc: Jacopo Mondi , Laurent Pinchart Subject: [PATCH v3 1/8] libcamera: base: log: Remove move constructor Message-ID: <20250217185433.306833-2-pobrn@protonmail.com> In-Reply-To: <20250217185433.306833-1-pobrn@protonmail.com> References: <20250217185433.306833-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 2169fabd493577edbd1c08ac8d713c493f96e11d MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" C++17 guarantees move and copy elision in certain cases, such as when returning a prvalue of the same type as the return type of the function. This is what the `_log()` functions do, thus there is no need for the move constructor, so remove it. Furthermore, do not just remove the implementation, but instead delete it as well. Signed-off-by: Barnabás Pőcze Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/base/log.h | 4 +--- src/libcamera/base/log.cpp | 19 ------------------- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h index cfe26f883..7a101d8ba 100644 --- a/include/libcamera/base/log.h +++ b/include/libcamera/base/log.h @@ -61,8 +61,6 @@ public: LogMessage(const char *fileName, unsigned int line, const LogCategory &category, LogSeverity severity, const std::string &prefix = std::string()); - - LogMessage(LogMessage &&); ~LogMessage(); [[nodiscard]] std::ostream &stream() { return msgStream_; } @@ -75,7 +73,7 @@ public: const std::string msg() const { return msgStream_.str(); } private: - LIBCAMERA_DISABLE_COPY(LogMessage) + LIBCAMERA_DISABLE_COPY_AND_MOVE(LogMessage) void init(const char *fileName, unsigned int line); diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index fd1427d0b..8c72a005d 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -866,25 +866,6 @@ LogMessage::LogMessage(const char *fileName, unsigned int line, init(fileName, line); } -/** - * \brief Move-construct a log message - * \param[in] other The other message - * - * The move constructor is meant to support the _log() functions. Thanks to copy - * elision it will likely never be called, but C++11 only permits copy elision, - * it doesn't enforce it unlike C++17. To avoid potential link errors depending - * on the compiler type and version, and optimization level, the move - * constructor is defined even if it will likely never be called, and ensures - * that the destructor of the \a other message will not output anything to the - * log by setting the severity to LogInvalid. - */ -LogMessage::LogMessage(LogMessage &&other) - : msgStream_(std::move(other.msgStream_)), category_(other.category_), - severity_(other.severity_) -{ - other.severity_ = LogInvalid; -} - void LogMessage::init(const char *fileName, unsigned int line) { /* Log the timestamp, severity and file information. */ From patchwork Mon Feb 17 18:54:45 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22802 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id B9A8EC3200 for ; Mon, 17 Feb 2025 18:54:51 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7286668686; Mon, 17 Feb 2025 19:54:51 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="erP1GLnY"; dkim-atps=neutral Received: from mail-40134.protonmail.ch (mail-40134.protonmail.ch [185.70.40.134]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A4D56867F for ; Mon, 17 Feb 2025 19:54:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1739818489; x=1740077689; bh=PYr8O0x9BteBWkfQs3r5XTedGHlewmyEL4aOauG2Ju8=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=erP1GLnYqknhVZHG4cm+Asok0Pvzzf1U+5l5Tt/4EoGMC7GEO1DNaT2EtSi6cHWUf JF2VDnHDHgVZgdUPlvSHzx/2gJJm6aqua4oNt3rHbbP1FheBPI2hAzTFYuzczOZFiB 8yV4sO4tgZascan6QPn9HOWiNGpOylFXgiPkPZAeENUCFgr8yGPKiTLU/pBMWp7+io yRRQ6j56SQE0Q4i2tlxZBVat5/tkWYcj3R8B3PRqhPx3O7y5J6fp1oK7p5d83bDG+q FI5a1AXg+3AU8s9Bhn6JKTdAXSVhEgjsnndSie9Piq/XWA5AThiPSI3ai+UskWwFZi /uWd00soay1nA== Date: Mon, 17 Feb 2025 18:54:45 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Cc: Laurent Pinchart , Jacopo Mondi Subject: [PATCH v3 2/8] libcamera: base: log: Use `std::from_chars()` Message-ID: <20250217185433.306833-3-pobrn@protonmail.com> In-Reply-To: <20250217185433.306833-1-pobrn@protonmail.com> References: <20250217185433.306833-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: a9381b72093bdc970382c6f3f22dfb8228ea5d5f MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Use the `std::from_chars()` function from `` to parse the integral log level instead of `strtoul` as it provides an easier to use interface and better type safety. Signed-off-by: Barnabás Pőcze Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- src/libcamera/base/log.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index 8c72a005d..3dea1e178 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -686,12 +687,11 @@ LogSeverity Logger::parseLogLevel(const std::string &level) "FATAL", }; - int severity; + unsigned int severity; if (std::isdigit(level[0])) { - char *endptr; - severity = strtoul(level.c_str(), &endptr, 10); - if (*endptr != '\0' || severity > LogFatal) + auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity); + if (ec != std::errc() || *end != '\0' || severity > LogFatal) severity = LogInvalid; } else { severity = LogInvalid; From patchwork Mon Feb 17 18:54:49 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22803 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 07163C3200 for ; Mon, 17 Feb 2025 18:54:56 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 76B8F6868D; Mon, 17 Feb 2025 19:54:55 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="VVT/WXpi"; dkim-atps=neutral Received: from mail-4316.protonmail.ch (mail-4316.protonmail.ch [185.70.43.16]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 331BA68686 for ; Mon, 17 Feb 2025 19:54:54 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1739818493; x=1740077693; bh=5vyz01HE8R9kF9A2i0e63Yfy0LD6JIXY9yK7jfVlbOM=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=VVT/WXpiO7lNSFbquA7Li0StFetS4YQAejJzsnOsVjz6CEIVMrm0w0NMRuq1m2RCQ tWsZVPLwic3L/Ye1QCJ0Z91x5h6DUH8gcqSo44DnlGeuufnSRW0huQZghTQGtQg0h2 dE4A11XnzIEVPecvOjP/7abDvwQOMg1t8oHNbGmR6EsHxgFSIasAD7/2OdotyOC0fg wiKenSu5WPUdEUDrtn9kO6q1jn6iGcSTB+YOgtMs5p38WR/YvmeLg0swVgD/RLDUCS /gdjOuIKY3ZgAY6o3OUyZKeIaosz9En8RfXzy36lI5F+l16khf0jyXdQ5ut+L32sKO TyxghHKfhCwJw== Date: Mon, 17 Feb 2025 18:54:49 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Cc: Laurent Pinchart Subject: [PATCH v3 3/8] libcamera: base: log: Remove `LogMessage::init()` Message-ID: <20250217185433.306833-4-pobrn@protonmail.com> In-Reply-To: <20250217185433.306833-1-pobrn@protonmail.com> References: <20250217185433.306833-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 9e969836c638949c3fed503d512c1490d8dbfebd MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" It is a short function that can be merged into the constructor with essentially no change in observable behaviour, so do that. Signed-off-by: Barnabás Pőcze Reviewed-by: Laurent Pinchart --- include/libcamera/base/log.h | 2 -- src/libcamera/base/log.cpp | 16 ++++------------ 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h index 7a101d8ba..66f665905 100644 --- a/include/libcamera/base/log.h +++ b/include/libcamera/base/log.h @@ -75,8 +75,6 @@ public: private: LIBCAMERA_DISABLE_COPY_AND_MOVE(LogMessage) - void init(const char *fileName, unsigned int line); - std::ostringstream msgStream_; const LogCategory &category_; LogSeverity severity_; diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index 3dea1e178..7043be711 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -861,19 +861,11 @@ const LogCategory &LogCategory::defaultCategory() LogMessage::LogMessage(const char *fileName, unsigned int line, const LogCategory &category, LogSeverity severity, const std::string &prefix) - : category_(category), severity_(severity), prefix_(prefix) + : category_(category), severity_(severity), + timestamp_(utils::clock::now()), + fileInfo_((std::ostringstream() << utils::basename(fileName) << ":" << line).str()), + prefix_(prefix) { - init(fileName, line); -} - -void LogMessage::init(const char *fileName, unsigned int line) -{ - /* Log the timestamp, severity and file information. */ - timestamp_ = utils::clock::now(); - - std::ostringstream ossFileInfo; - ossFileInfo << utils::basename(fileName) << ":" << line; - fileInfo_ = ossFileInfo.str(); } LogMessage::~LogMessage() From patchwork Mon Feb 17 18:54:54 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22804 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 1DC30C3200 for ; Mon, 17 Feb 2025 18:55:01 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BE5BC68691; Mon, 17 Feb 2025 19:55:00 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="fcYnlJQX"; dkim-atps=neutral Received: from mail-40131.protonmail.ch (mail-40131.protonmail.ch [185.70.40.131]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D2B7E6867F for ; Mon, 17 Feb 2025 19:54:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1739818498; x=1740077698; bh=APHFt1qeWUYoQjFwTkfxA7g56I/2fDczwSkYYslrItc=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=fcYnlJQXvmd3NIYOPStfGFivrMpTDC6QizWcFOYC9nHiZjlH0DrDjR2bC0/k/XZe8 VyTslx5IJLaGqLfk4vxLGHeJ7+l37kkYDwRzy5icH67rq3yrpPnJFrYF7LvxdZMxGs Jn/XSipETtvP87QR7bh7EJkttbkWJZCdUoTZenvWsWmW2rUnKGU1uouu+xllkgt4zX VNnDqfBwbhxj7JqmIk8Qv8Xo8Y0YhLncy3pXF+bvaU/vhnODcVevUrjRyTciKOi+RW ljjS5lVeo6T9DWzkxrMwal/HOtwpeo1v1ita5a2VWDG8VliVW3MnhPk1XoT0AVZdFW wQUVIYDNlkYwQ== Date: Mon, 17 Feb 2025 18:54:54 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Cc: Laurent Pinchart , Jacopo Mondi Subject: [PATCH v3 4/8] libcamera: base: log: Make `LogCategory::severity_` atomic Message-ID: <20250217185433.306833-5-pobrn@protonmail.com> In-Reply-To: <20250217185433.306833-1-pobrn@protonmail.com> References: <20250217185433.306833-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 23c4234b7c99a5eac8b19e151ac2b3f4c0f7bf49 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The severity of a log category may be changed from a different thread, so it is important to ensure that the reads and writes happen atomically. Using `std::memory_order_relaxed` should not introduce any synchronization overhead, it should only guarantee that the operation itself is atomic. Secondly, inline `LogCategory::setSeverity()`, as it is merely an assignment, so going through a DSO call is a big pessimization. `LogCategory` is not part of the public API, so this change has no external effects. Thirdly, assert that the atomic variable is lock free so as to ensure it won't silently fall back to libatomic (or similar) on any platform. If this assertion fails, this needs to be revisited. Signed-off-by: Barnabás Pőcze Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- include/libcamera/base/log.h | 9 ++++++--- src/libcamera/base/log.cpp | 5 +---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h index 66f665905..6c809e38a 100644 --- a/include/libcamera/base/log.h +++ b/include/libcamera/base/log.h @@ -7,6 +7,7 @@ #pragma once +#include #include #include @@ -31,8 +32,8 @@ public: static LogCategory *create(const char *name); const std::string &name() const { return name_; } - LogSeverity severity() const { return severity_; } - void setSeverity(LogSeverity severity); + LogSeverity severity() const { return severity_.load(std::memory_order_relaxed); } + void setSeverity(LogSeverity severity) { severity_.store(severity, std::memory_order_relaxed); } static const LogCategory &defaultCategory(); @@ -40,7 +41,9 @@ private: explicit LogCategory(const char *name); const std::string name_; - LogSeverity severity_; + + std::atomic severity_; + static_assert(decltype(severity_)::is_always_lock_free); }; #define LOG_DECLARE_CATEGORY(name) \ diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index 7043be711..e2c2eea8f 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -810,15 +810,12 @@ LogCategory::LogCategory(const char *name) */ /** + * \fn LogCategory::setSeverity(LogSeverity severity) * \brief Set the severity of the log category * * Messages of severity higher than or equal to the severity of the log category * are printed, other messages are discarded. */ -void LogCategory::setSeverity(LogSeverity severity) -{ - severity_ = severity; -} /** * \brief Retrieve the default log category From patchwork Mon Feb 17 18:54:57 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22805 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 2954BC3200 for ; Mon, 17 Feb 2025 18:55:04 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9C15C68691; Mon, 17 Feb 2025 19:55:03 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="I1Rccz3p"; dkim-atps=neutral Received: from mail-10630.protonmail.ch (mail-10630.protonmail.ch [79.135.106.30]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D9BE468692 for ; Mon, 17 Feb 2025 19:55:00 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1739818500; x=1740077700; bh=EcehzJZziSdPo3Ijxc5UQcDipLGaRFBXUeBDp59Km64=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=I1Rccz3pOv1Ut4POGK+neAupSynnr81foE89lhsYFT3CfqrDLOml4QdYQpf4UfZ2e EgAiC1vuo8sMSAoXShv1nGnNRCUTc7jLXb/Fw2cyMsIVMylvL9N5QHKw8HpLWwuerr CP7ijysBaFw6Lm86KEVqUFDpX0ZE4Tra/D66RyweDb+/fISn3xtU1GEoCcjLLpddrd 5edf7HGqaBpVLIOYuYn+FHQ/OFHgwy8+c9mE2yJpVRX4WQvrRoVtHBAJy+5wtTB4vG CnnuPlIS+ocosID94x2oyLkWrzgtSXrq3B9GHFh5i49eMbFURU7+aLyCNmj4dqqK4c ZYvf0kkT8Q7mQ== Date: Mon, 17 Feb 2025 18:54:57 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Cc: Laurent Pinchart , Jacopo Mondi Subject: [PATCH v3 5/8] libcamera: base: log: Use `std::string_view` to avoid some copies Message-ID: <20250217185433.306833-6-pobrn@protonmail.com> In-Reply-To: <20250217185433.306833-1-pobrn@protonmail.com> References: <20250217185433.306833-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 950dfeddbbfdcf737fa036b98152107f8da2558c MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Use `std::string_view` to avoid some largely unnecessary copies, and to make string comparisong potentially faster by eliminating repeated `strlen()` calls. Signed-off-by: Barnabás Pőcze Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- include/libcamera/base/log.h | 5 +++-- src/libcamera/base/log.cpp | 25 +++++++++++++------------ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h index 6c809e38a..353520e10 100644 --- a/include/libcamera/base/log.h +++ b/include/libcamera/base/log.h @@ -9,6 +9,7 @@ #include #include +#include #include @@ -29,7 +30,7 @@ enum LogSeverity { class LogCategory { public: - static LogCategory *create(const char *name); + static LogCategory *create(std::string_view name); const std::string &name() const { return name_; } LogSeverity severity() const { return severity_.load(std::memory_order_relaxed); } @@ -38,7 +39,7 @@ public: static const LogCategory &defaultCategory(); private: - explicit LogCategory(const char *name); + explicit LogCategory(std::string_view name); const std::string name_; diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index e2c2eea8f..55019def8 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -313,11 +314,11 @@ private: void parseLogFile(); void parseLogLevels(); - static LogSeverity parseLogLevel(const std::string &level); + static LogSeverity parseLogLevel(std::string_view level); friend LogCategory; void registerCategory(LogCategory *category); - LogCategory *findCategory(const char *name) const; + LogCategory *findCategory(std::string_view name) const; static bool destroyed_; @@ -642,17 +643,17 @@ void Logger::parseLogLevels() if (!len) continue; - std::string category; - std::string level; + std::string_view category; + std::string_view level; const char *colon = static_cast(memchr(pair, ':', len)); if (!colon) { /* 'x' is a shortcut for '*:x'. */ category = "*"; - level = std::string(pair, len); + level = std::string_view(pair, len); } else { - category = std::string(pair, colon - pair); - level = std::string(colon + 1, comma - colon - 1); + category = std::string_view(pair, colon - pair); + level = std::string_view(colon + 1, comma - colon - 1); } /* Both the category and the level must be specified. */ @@ -663,7 +664,7 @@ void Logger::parseLogLevels() if (severity == LogInvalid) continue; - levels_.push_back({ category, severity }); + levels_.emplace_back(category, severity); } } @@ -677,7 +678,7 @@ void Logger::parseLogLevels() * * \return The log severity, or LogInvalid if the string is invalid */ -LogSeverity Logger::parseLogLevel(const std::string &level) +LogSeverity Logger::parseLogLevel(std::string_view level) { static const char *const names[] = { "DEBUG", @@ -729,7 +730,7 @@ void Logger::registerCategory(LogCategory *category) * \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 +LogCategory *Logger::findCategory(std::string_view name) const { if (auto it = std::find_if(categories_.begin(), categories_.end(), [name](auto c) { return c->name() == name; }); @@ -773,7 +774,7 @@ LogCategory *Logger::findCategory(const char *name) const * * \return The pointer to the LogCategory */ -LogCategory *LogCategory::create(const char *name) +LogCategory *LogCategory::create(std::string_view name) { static Mutex mutex_; MutexLocker locker(mutex_); @@ -791,7 +792,7 @@ LogCategory *LogCategory::create(const char *name) * \brief Construct a log category * \param[in] name The category name */ -LogCategory::LogCategory(const char *name) +LogCategory::LogCategory(std::string_view name) : name_(name), severity_(LogSeverity::LogInfo) { } From patchwork Mon Feb 17 18:55:02 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22806 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 70BA6C3200 for ; Mon, 17 Feb 2025 18:55:10 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2129C68696; Mon, 17 Feb 2025 19:55:10 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="Y7pOS9Hq"; dkim-atps=neutral Received: from mail-40134.protonmail.ch (mail-40134.protonmail.ch [185.70.40.134]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0B7276868D for ; Mon, 17 Feb 2025 19:55:08 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1739818507; x=1740077707; bh=FV4vPqZlU6kReFob6wa3fv269jNCccPDdKLA4RCB100=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=Y7pOS9HqTiFEuBl2IjRIlb0GT/rj4RkqMGzELVpAHYSM1Mr8pAE8+SqsfkLl+e+ZV 7udH0CXMf19ozD6CXpWKsNcjO/z0kd+jf4K1r8S83InVrKzUvLuVy+8z9KZ9LsNMAr EMWUqAlsW3t47A0BKk9uCrQB8O8bQacxILdOAANOvG9yd2qasxoFm34OP41zqc6EBv 0dGU3/xCKZ5lv3/5dYKxWmPEke7vxX6ckVHMMKoi9frfUTa74+EX8TWWaMhGrV009r JpqzxkIqLo90Y64ybhr3W+9VRpkxoAmzkIi55KdNzCpNRTG2PaqT2CyGoRdQoijCIh c3cymiP/MdVuQ== Date: Mon, 17 Feb 2025 18:55:02 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Cc: Laurent Pinchart Subject: [PATCH v3 6/8] libcamera: base: log: Pass dynamic prefix through Message-ID: <20250217185433.306833-7-pobrn@protonmail.com> In-Reply-To: <20250217185433.306833-1-pobrn@protonmail.com> References: <20250217185433.306833-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 86fd220242882396c841d43bb447aa34fa478d82 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Use move construction to essentially pass through the string returned by `Loggable::logPrefix()` to avoid an unnecessary copy. Signed-off-by: Barnabás Pőcze Reviewed-by: Laurent Pinchart --- include/libcamera/base/log.h | 2 +- src/libcamera/base/log.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h index 353520e10..195426c41 100644 --- a/include/libcamera/base/log.h +++ b/include/libcamera/base/log.h @@ -64,7 +64,7 @@ class LogMessage public: LogMessage(const char *fileName, unsigned int line, const LogCategory &category, LogSeverity severity, - const std::string &prefix = std::string()); + std::string prefix = {}); ~LogMessage(); [[nodiscard]] std::ostream &stream() { return msgStream_; } diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index 55019def8..2c233be36 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -858,11 +858,11 @@ const LogCategory &LogCategory::defaultCategory() */ LogMessage::LogMessage(const char *fileName, unsigned int line, const LogCategory &category, LogSeverity severity, - const std::string &prefix) + std::string prefix) : category_(category), severity_(severity), timestamp_(utils::clock::now()), fileInfo_((std::ostringstream() << utils::basename(fileName) << ":" << line).str()), - prefix_(prefix) + prefix_(std::move(prefix)) { } From patchwork Mon Feb 17 18:55:05 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22807 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id C61EDC3200 for ; Mon, 17 Feb 2025 18:55:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1E38A68695; Mon, 17 Feb 2025 19:55:11 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="YfcyLUDG"; dkim-atps=neutral Received: from mail-4322.protonmail.ch (mail-4322.protonmail.ch [185.70.43.22]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 66A386868F for ; Mon, 17 Feb 2025 19:55:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1739818508; x=1740077708; bh=pqomrIDtjkGyEkLgEq9/KVdSaTtPwV82mzxoAhFP/us=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=YfcyLUDGSYwkL560bSoRx6XVGJD2uxSaVZWu8eD4IKX/VQ7Gn5FxUeF/q4MdNklfv EUthgkAFrzDRZRBMuxSYeQWsyabXMJ5aSzXN3xXOQqQVxCvtC4onaK+NqRpKK3Jgm5 AoX9KbUZwTSlnoiMuP2oRQhxZ0mBzuM/ixFK9gjqUk1ERlK+gZN2dM2QeDGWGLsf9z 3lmn0fuVBxhMOwrY2b1fdPBYXkCTQft/S6ZJGm7vQvhcdL7gzhqvsAN57lGJiZyobL roaQzivh03Cs6VZsO9eScwV1AufJxd1vqZPdvtCaeuYlxj+LejNoulAQ/W6m64y9G1 5f/xSpBjTMBgg== Date: Mon, 17 Feb 2025 18:55:05 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Cc: Jacopo Mondi Subject: [PATCH v3 7/8] libcamera: base: log: Protect log categories with lock Message-ID: <20250217185433.306833-8-pobrn@protonmail.com> In-Reply-To: <20250217185433.306833-1-pobrn@protonmail.com> References: <20250217185433.306833-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 2856231cfd9a8cdce6b6b60b560d4693ebd08e90 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Log categories may be added from any thread, so it is important to synchronize access to the `Logger::categories_` list between its two users: category creation (by LogCategory::create(), which calls Logger::findCategory() and Logger::registerCategory()); and log level setting (by Logger::logSetLevel()). The LogCategory::create() function uses a mutex to serialize category creation, but Logger::logSetLevel() can access `Logger::categories_` concurrently without any protection. To fix the issue, move the mutex to the Logger class, and use it to protect all accesses to the categories list. This requires moving all the logic of LogCategory::create() to a new Logger::findOrCreateCategory() function that combines both Logger::findCategory() and Logger::registerCategory() in order to make the two operations exacute atomically. Signed-off-by: Barnabás Pőcze Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/base/log.h | 1 + src/libcamera/base/log.cpp | 58 +++++++++++++----------------------- 2 files changed, 22 insertions(+), 37 deletions(-) diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h index 195426c41..8ae1b51d0 100644 --- a/include/libcamera/base/log.h +++ b/include/libcamera/base/log.h @@ -39,6 +39,7 @@ public: static const LogCategory &defaultCategory(); private: + friend class Logger; explicit LogCategory(std::string_view name); const std::string name_; diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index 2c233be36..fd6c11716 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -317,12 +317,12 @@ private: static LogSeverity parseLogLevel(std::string_view level); friend LogCategory; - void registerCategory(LogCategory *category); - LogCategory *findCategory(std::string_view name) const; + LogCategory *findOrCreateCategory(std::string_view name); static bool destroyed_; - std::vector categories_; + Mutex mutex_; + std::vector categories_ LIBCAMERA_TSA_GUARDED_BY(mutex_); std::list> levels_; std::atomic> output_; @@ -572,6 +572,8 @@ void Logger::logSetLevel(const char *category, const char *level) if (severity == LogInvalid) return; + MutexLocker locker(mutex_); + for (LogCategory *c : categories_) { if (c->name() == category) { c->setSeverity(severity); @@ -708,37 +710,28 @@ LogSeverity Logger::parseLogLevel(std::string_view level) } /** - * \brief Register a log category with the logger - * \param[in] category The log category - * - * Log categories must have unique names. It is invalid to call this function - * if a log category with the same name already exists. + * \brief Find an existing log category with the given name or create one + * \param[in] name Name of the log category + * \return The pointer to the log category found or created */ -void Logger::registerCategory(LogCategory *category) +LogCategory *Logger::findOrCreateCategory(std::string_view name) { - categories_.push_back(category); + MutexLocker locker(mutex_); - const std::string &name = category->name(); - for (const auto &[pattern, severity] : levels_) { - if (fnmatch(pattern.c_str(), name.c_str(), FNM_NOESCAPE) == 0) - category->setSeverity(severity); + for (LogCategory *category : categories_) { + if (category->name() == name) + return 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(std::string_view name) const -{ - if (auto it = std::find_if(categories_.begin(), categories_.end(), - [name](auto c) { return c->name() == name; }); - it != categories_.end()) { - return *it; + LogCategory *category = categories_.emplace_back(new LogCategory(name)); + const char *name_cstr = category->name().c_str(); + + for (const auto &[pattern, severity] : levels_) { + if (fnmatch(pattern.c_str(), name_cstr, FNM_NOESCAPE) == 0) + category->setSeverity(severity); } - return nullptr; + return category; } /** @@ -776,16 +769,7 @@ LogCategory *Logger::findCategory(std::string_view name) const */ LogCategory *LogCategory::create(std::string_view name) { - static Mutex mutex_; - MutexLocker locker(mutex_); - LogCategory *category = Logger::instance()->findCategory(name); - - if (!category) { - category = new LogCategory(name); - Logger::instance()->registerCategory(category); - } - - return category; + return Logger::instance()->findOrCreateCategory(name); } /** From patchwork Mon Feb 17 18:55:10 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22808 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 0CC13C3200 for ; Mon, 17 Feb 2025 18:55:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A955468698; Mon, 17 Feb 2025 19:55:15 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="WMewOK2l"; dkim-atps=neutral Received: from mail-40133.protonmail.ch (mail-40133.protonmail.ch [185.70.40.133]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 98DF368697 for ; Mon, 17 Feb 2025 19:55:12 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1739818512; x=1740077712; bh=IKmqx1SXJwpufJa0slW3yC48eFAvkOWt4qFhcYH/H+g=; h=Date:To:From:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=WMewOK2lrcwyfaC6Q0wOX92dmRLGt93KxjOHnmySye0Z/kGdEQN+2RJouFjoXIdg/ jxuslklxlcQQaqAlXa5BClHr9DNIarxk6Q02EM2PYHfoKe8+Wtb1KEfKZsWclcipZD q6r4WY1px3hlnI5/yi3P5R9o3uFExu4IEXOAFVpha0Lgjh9nfXTGl9AQ2RZSdcgg3J WnsgE5GD4JE9qt+Qo8r8w9nZXWD8/jUdtHj74ptDE0fD7K6vUUP4lv+KYMd3248dgg mMnqk39qxNAwgPIKwa8A5I/lxv2xXblzlk/hMJAVObXUWg2ohKLrc56ucbp+E/vWyF 25ddKoox9Zx9A== Date: Mon, 17 Feb 2025 18:55:10 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [PATCH v3 8/8] libcamera: base: log: Avoid manual `LogCategory` deletion Message-ID: <20250217185433.306833-9-pobrn@protonmail.com> In-Reply-To: <20250217185433.306833-1-pobrn@protonmail.com> References: <20250217185433.306833-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 56ff5397914f1c91dccec090acdcd645003fd2c7 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Wrap the `LogCategory` pointers in `std::unique_ptr` to avoid the need for manual deletion in the destructor. Signed-off-by: Barnabás Pőcze Reviewed-by: Laurent Pinchart --- src/libcamera/base/log.cpp | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index fd6c11716..e05ffc1c4 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -322,7 +322,7 @@ private: static bool destroyed_; Mutex mutex_; - std::vector categories_ LIBCAMERA_TSA_GUARDED_BY(mutex_); + std::vector> categories_ LIBCAMERA_TSA_GUARDED_BY(mutex_); std::list> levels_; std::atomic> output_; @@ -439,9 +439,6 @@ void logSetLevel(const char *category, const char *level) Logger::~Logger() { destroyed_ = true; - - for (LogCategory *category : categories_) - delete category; } /** @@ -574,7 +571,7 @@ void Logger::logSetLevel(const char *category, const char *level) MutexLocker locker(mutex_); - for (LogCategory *c : categories_) { + for (const auto &c : categories_) { if (c->name() == category) { c->setSeverity(severity); break; @@ -718,12 +715,12 @@ LogCategory *Logger::findOrCreateCategory(std::string_view name) { MutexLocker locker(mutex_); - for (LogCategory *category : categories_) { + for (const auto &category : categories_) { if (category->name() == name) - return category; + return category.get(); } - LogCategory *category = categories_.emplace_back(new LogCategory(name)); + LogCategory *category = categories_.emplace_back(std::unique_ptr(new LogCategory(name))).get(); const char *name_cstr = category->name().c_str(); for (const auto &[pattern, severity] : levels_) {