[{"id":37732,"web_url":"https://patchwork.libcamera.org/comment/37732/","msgid":"<176882712371.3486172.2586969758901113508@ping.linuxembedded.co.uk>","date":"2026-01-19T12:52:03","subject":"Re: [PATCH v2 2/5] libcamera: base: log: Do not instantiate disabled\n\t`LogMessage`s","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2026-01-19 11:31:00)\n> At the moment every `LOG()` macro invocation results in a `LogMessage` being\n> created, the message serialized into an `std::stringstream`. Only in the\n> destructor is it actually checked whether the given `LogCategory` enables\n> the given log level.\n> \n> This is not too efficient, it would be better to skip the log message\n> construction and all the `operator<<()` invocations if the message will\n> just be discarded.\n> \n> This could be easily done if the `LOG()` macro accepted its arguments like a\n> traditional function as in that case an appropriate `if` statement can be\n> injected in a do-while loop. However, that is not the case, the `LOG()` macro\n> should effectively \"return\" a stream.\n> \n> It is not possible inject an `if` statement directly as that would\n> lead to issues:\n> \n> if (...)\n> LOG(...)\n> else\n> ...\n> \n> The `else` would bind the to the `if` in the `LOG()` macro. This is\n> diagnosed by `-Wdangling-else`.\n> \n> An alternative approach would be to use a `for` loop and force a single\n> iteration using a boolean flag or similar. This is entirely doable but\n> I think the implemented approach is easier to understand.\n> \n> This change implements the early log level checking using a `switch` statement\n> as this avoids the dangling else related issues. One small issue arises\n> because having a boolean controlling expression is diagnosed by clang\n> (`-Wswitch-bool`); the result is cast to `int` to avoid the warning.\n\nI can live with that.\n\nReviewed-by: Kieran Bingham \n\n> \n> Signed-off-by: Barnabás Pőcze \n> ---\n> include/libcamera/base/log.h | 9 +++++++--\n> 1 file changed, 7 insertions(+), 2 deletions(-)\n> \n> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h\n> index d8338d8df..dbd14951c 100644\n> --- a/include/libcamera/base/log.h\n> +++ b/include/libcamera/base/log.h\n> @@ -108,10 +108,15 @@ LogMessage _log(const LogCategory &category, LogSeverity severity,\n> #ifndef __DOXYGEN__\n> #define _LOG_CATEGORY(name) logCategory##name\n> \n> +#define _LOG(cat, sev) \\\n> + switch (const auto &_logCategory = (cat); int(_logCategory.severity() <= Log##sev)) \\\n> + case 1: \\\n> + _log(_logCategory, Log##sev).stream()\n> +\n> #define _LOG1(severity) \\\n> - _log(LogCategory::defaultCategory(), Log##severity).stream()\n> + _LOG(LogCategory::defaultCategory(), severity)\n> #define _LOG2(category, severity) \\\n> - _log(_LOG_CATEGORY(category)(), Log##severity).stream()\n> + _LOG(_LOG_CATEGORY(category)(), severity)\n> \n> /*\n> * Expand the LOG() macro to _LOG1() or _LOG2() based on the number of\n> -- \n> 2.52.0\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 98366BDCBF\n\tfor ;\n\tMon, 19 Jan 2026 12:52:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4489D61FBC;\n\tMon, 19 Jan 2026 13:52:07 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7E011606D5\n\tfor ;\n\tMon, 19 Jan 2026 13:52:06 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 285A4B3;\n\tMon, 19 Jan 2026 13:51:36 +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=\"r4+sCs/Z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768827096;\n\tbh=SapbSSF2x6Oo1BXfIoGJoZ+wH3yYj4HpXNjRh1T1W+o=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=r4+sCs/ZBCVgu4NAu4O7pWaBVt0V8/zKitlD+wQsS9mTEfy/Bkq1QZkvBfwCVVqy8\n\twAD+wTaH9rb+jw1ubBBplaVbImY+2zjF8iiR/vhvCo6rd127BsKmPnrYOFuRuy+OJ8\n\t5a9Jzf4VvHF16KzqmBclmSSk0JHAMDe/X6CnvPec=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260119113104.3560802-3-barnabas.pocze@ideasonboard.com>","References":"<20260119113104.3560802-1-barnabas.pocze@ideasonboard.com>\n\t<20260119113104.3560802-3-barnabas.pocze@ideasonboard.com>","Subject":"Re: [PATCH v2 2/5] libcamera: base: log: Do not instantiate disabled\n\t`LogMessage`s","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 19 Jan 2026 12:52:03 +0000","Message-ID":"<176882712371.3486172.2586969758901113508@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":37746,"web_url":"https://patchwork.libcamera.org/comment/37746/","msgid":"<20260119174016.GE26642@pendragon.ideasonboard.com>","date":"2026-01-19T17:40:16","subject":"Re: [PATCH v2 2/5] libcamera: base: log: Do not instantiate disabled\n\t`LogMessage`s","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 Mon, Jan 19, 2026 at 12:31:00PM +0100, Barnabás Pőcze wrote:\n> At the moment every `LOG()` macro invocation results in a `LogMessage` being\n> created, the message serialized into an `std::stringstream`. Only in the\n> destructor is it actually checked whether the given `LogCategory` enables\n> the given log level.\n> \n> This is not too efficient, it would be better to skip the log message\n> construction and all the `operator<<()` invocations if the message will\n> just be discarded.\n> \n> This could be easily done if the `LOG()` macro accepted its arguments like a\n> traditional function as in that case an appropriate `if` statement can be\n> injected in a do-while loop. However, that is not the case, the `LOG()` macro\n> should effectively \"return\" a stream.\n> \n> It is not possible inject an `if` statement directly as that would\n> lead to issues:\n> \n> if (...)\n> LOG(...)\n> else\n> ...\n> \n> The `else` would bind the to the `if` in the `LOG()` macro. This is\n> diagnosed by `-Wdangling-else`.\n> \n> An alternative approach would be to use a `for` loop and force a single\n> iteration using a boolean flag or similar. This is entirely doable but\n> I think the implemented approach is easier to understand.\n> \n> This change implements the early log level checking using a `switch` statement\n> as this avoids the dangling else related issues. One small issue arises\n> because having a boolean controlling expression is diagnosed by clang\n> (`-Wswitch-bool`); the result is cast to `int` to avoid the warning.\n> \n> Signed-off-by: Barnabás Pőcze \n> ---\n> include/libcamera/base/log.h | 9 +++++++--\n> 1 file changed, 7 insertions(+), 2 deletions(-)\n> \n> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h\n> index d8338d8df..dbd14951c 100644\n> --- a/include/libcamera/base/log.h\n> +++ b/include/libcamera/base/log.h\n> @@ -108,10 +108,15 @@ LogMessage _log(const LogCategory &category, LogSeverity severity,\n> #ifndef __DOXYGEN__\n> #define _LOG_CATEGORY(name) logCategory##name\n> \n> +#define _LOG(cat, sev) \\\n> +\tswitch (const auto &_logCategory = (cat); int(_logCategory.severity() <= Log##sev)) \\\n\nstatic_cast()\n\nI still worry a bit about log statements with side effects (worrying a\nbit about things seem to be part of my job description), but I have to\nagree it's extremely unlikely. That's why we have reviews.\n\nReviewed-by: Laurent Pinchart \n\n> +\tcase 1: \\\n> +\t\t_log(_logCategory, Log##sev).stream()\n> +\n> #define _LOG1(severity) \\\n> -\t_log(LogCategory::defaultCategory(), Log##severity).stream()\n> +\t_LOG(LogCategory::defaultCategory(), severity)\n> #define _LOG2(category, severity) \\\n> -\t_log(_LOG_CATEGORY(category)(), Log##severity).stream()\n> +\t_LOG(_LOG_CATEGORY(category)(), severity)\n> \n> /*\n> * Expand the LOG() macro to _LOG1() or _LOG2() based on the number of","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 6BEB2C3220\n\tfor ;\n\tMon, 19 Jan 2026 17:40:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9774D61FC6;\n\tMon, 19 Jan 2026 18:40:41 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5ADB4606D5\n\tfor ;\n\tMon, 19 Jan 2026 18:40:39 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-152.bb.dnainternet.fi\n\t[81.175.209.152])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 82A09C1;\n\tMon, 19 Jan 2026 18:40:08 +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=\"py6nnl7P\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768844408;\n\tbh=BXu8moDYlk70hBbh3MWKUIF+gm8wXAoQB4IUyTAsCNo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=py6nnl7PIlpZ5kS7vzvVCkssRYh/VsAOKMTLVY+Titf6zUEtTWOSCy9QOajx3204E\n\tr+UZggUkKEO+2h+e+H/w7P9X/5evZBctBMWmAMS8iiI3cUndaH1/RUUZQVkf1Bpo9d\n\t5K49HcCVwBXTAvg3MonHFTp9j6QRUxREqbUJpQe8=","Date":"Mon, 19 Jan 2026 19:40:16 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 2/5] libcamera: base: log: Do not instantiate disabled\n\t`LogMessage`s","Message-ID":"<20260119174016.GE26642@pendragon.ideasonboard.com>","References":"<20260119113104.3560802-1-barnabas.pocze@ideasonboard.com>\n\t<20260119113104.3560802-3-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20260119113104.3560802-3-barnabas.pocze@ideasonboard.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\" "}}]