[{"id":34604,"web_url":"https://patchwork.libcamera.org/comment/34604/","msgid":"","date":"2025-06-06T09:11:03","subject":"Re: [PATCH] libcamera: base: Fix log level parsing when multiple\n\tcategories are listed","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 06. 06. 10:37 keltezéssel, Stefan Klug írta:\n> For a list of log levels like LIBCAMERA_LOG_LEVELS=\"CatA:0,CatB:1\" only\n> the severity of the last entry is correctly parsed.\n> \n> Due to the change of level to a string_view in 24c2caa1c1b3 (\"libcamera:\n> base: log: Use `std::string_view` to avoid some copies\") the level is no\n> longer necessarily null terminated as it is a view on the original data.\n> \n> Replace the check for a terminating null by a check for the end position\n> to fix the issue.\n\nOops, sorry. I hope it wasn't too hard to find.\n\n\n> \n> Fixes: 24c2caa1c1b3 (\"libcamera: base: log: Use `std::string_view` to avoid some copies\")\n> Signed-off-by: Stefan Klug \n> ---\n> src/libcamera/base/log.cpp | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> index 8bf3e1daa9c6..e024b58d7180 100644\n> --- a/src/libcamera/base/log.cpp\n> +++ b/src/libcamera/base/log.cpp\n> @@ -690,8 +690,9 @@ LogSeverity Logger::parseLogLevel(std::string_view level)\n> \tunsigned int severity = LogInvalid;\n> \n> \tif (std::isdigit(level[0])) {\n> -\t\tauto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);\n> -\t\tif (ec != std::errc() || *end != '\\0' || severity > LogFatal)\n> +\t\tconst char* levelEnd = level.data() + level.size();\n\nconst char *levelEnd ?\n\nCould a test be added? Maybe something like this:\n\n---\ndiff --git a/test/log/log_api.cpp b/test/log/log_api.cpp\nindex 0b999738d..8587ea30d 100644\n--- a/test/log/log_api.cpp\n+++ b/test/log/log_api.cpp\n@@ -26,10 +26,42 @@ using namespace std;\n using namespace libcamera;\n \n LOG_DEFINE_CATEGORY(LogAPITest)\n+LOG_DEFINE_CATEGORY(Cat0)\n+LOG_DEFINE_CATEGORY(Cat1)\n+LOG_DEFINE_CATEGORY(Cat2)\n+LOG_DEFINE_CATEGORY(Cat3)\n+LOG_DEFINE_CATEGORY(Cat4)\n \n class LogAPITest : public Test\n {\n protected:\n+\tint testEnvLevels()\n+\t{\n+\t\tsetenv(\"LIBCAMERA_LOG_LEVELS\",\n+\t\t \"Cat0:0,Cat0:9999,Cat1:INFO,Cat1:INVALID,Cat2:2,Cat2:-1,Cat3:ERROR,Cat3:{[]},Cat4:4,Cat4:rubbish\",\n+\t\t true);\n+\t\tlogSetTarget(libcamera::LoggingTargetNone);\n+\n+\t\tconst std::pair expected[] = {\n+\t\t\t{ _LOG_CATEGORY(Cat0)(), libcamera::LogDebug },\n+\t\t\t{ _LOG_CATEGORY(Cat1)(), libcamera::LogInfo },\n+\t\t\t{ _LOG_CATEGORY(Cat2)(), libcamera::LogWarning },\n+\t\t\t{ _LOG_CATEGORY(Cat3)(), libcamera::LogError },\n+\t\t\t{ _LOG_CATEGORY(Cat4)(), libcamera::LogFatal },\n+\t\t};\n+\t\tbool ok = true;\n+\n+\t\tfor (const auto &[c, s] : expected) {\n+\t\t\tif (c.severity() != s) {\n+\t\t\t\tok = false;\n+\t\t\t\tcerr << \"Severity of \" << c.name() << \" (\" << c.severity() << \") \"\n+\t\t\t\t << \"does not equal \" << s << endl;\n+\t\t\t}\n+\t\t}\n+\n+\t\treturn ok ? TestPass : TestFail;\n+\t}\n+\n \tvoid doLogging()\n \t{\n \t\tlogSetLevel(\"LogAPITest\", \"DEBUG\");\n@@ -135,7 +167,11 @@ protected:\n \n \tint run() override\n \t{\n-\t\tint ret = testFile();\n+\t\tint ret = testEnvLevels();\n+\t\tif (ret != TestPass)\n+\t\t\treturn TestFail;\n+\n+\t\tret = testFile();\n \t\tif (ret != TestPass)\n \t\t\treturn TestFail;\n \n---\n\nReviewed-by: Barnabás Pőcze \n\n\n> +\t\tauto [end, ec] = std::from_chars(level.data(), levelEnd, severity);\n> +\t\tif (ec != std::errc() || end != levelEnd || severity > LogFatal)\n> \t\t\tseverity = LogInvalid;\n> \t} else {\n> \t\tfor (unsigned int i = 0; i < std::size(names); ++i) {","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":["parsemail@patchwork.libcamera.org","kbingham@ideasonboard.com"],"Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3107DBDE6B\n\tfor ;\n\tMon, 23 Jun 2025 14:11:49 +0000 (UTC)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net\n\t[86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 53B4915BF\n\tfor ;\n\tMon, 23 Jun 2025 16:11:32 +0200 (CEST)","from perceval.ideasonboard.com\n\tby perceval.ideasonboard.com with LMTP id sOLbMaqwQmi/+x4A4E0KoQ\n\t(envelope-from )\n\tfor ; Fri, 06 Jun 2025 11:11:06 +0200","from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\tby perceval.ideasonboard.com (Postfix) with ESMTPS\n\tid 3DD3A593;\tFri, 6 Jun 2025 11:11:06 +0200 (CEST)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC8E168DBE;\n\tFri, 6 Jun 2025 11:11:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5F41368DB1\n\tfor ;\n\tFri, 6 Jun 2025 11:11:07 +0200 (CEST)","from [192.168.33.19] (185.182.214.22.nat.pool.zt.hu\n\t[185.182.214.22]) by perceval.ideasonboard.com (Postfix) with ESMTPSA\n\tid B597111DD; Fri, 6 Jun 2025 11:11:02 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750687892;\n\tbh=2SIY2qNQz3AuHMtCZqmgUBrENcujmG0aZHf3KNEltOg=;\n\th=Date:Subject:To:References:From:In-Reply-To:List-Id:\n\tList-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe:\n\tResent-From:Resent-To:From;\n\tb=mzEbRQv78EdcDNgpA5uMeVDGOjAYwQ9OERbrcNMcBHFlfKnVTJMP26c/ZKhLXTUvr\n\tFWkdGWdhIYvEBxlpPtWGJu5HQMtFq/cuRiSIbvf7fbgnJL/aG8a3UjbSRCH6oKOlJI\n\tZm7nSd7a8Las/El8J7ozkc5PMnvgSMe3z0GQHS+s=","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1749201062;\n\tbh=HtKfwh83qY8dFAxA7AqMcaYyy9sReGkatsyEPdHoJkQ=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=o0ypORZgqDdx9HB2WAKNUcOOhasthQNhjhwIidlwjZSN/oqd/xblyoRdDtjRra93e\n\tOBkys2WJNUD34W+MIjk04TWjGIRSWsOlPOX7Tp1dgtYMlsVNlTvhh5HV+aREtqY0Ar\n\tfU2TvhlQQ0DPfy46cK03qRuCJerltFCbXQs1d578="],"Authentication-Results":["perceval.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.a=rsa-sha256 header.s=mail header.b=o0ypORZg; dkim-atps=neutral","lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"o0ypORZg\";\tdkim-atps=neutral"],"Message-ID":"","Date":"Fri, 6 Jun 2025 11:11:03 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH] libcamera: base: Fix log level parsing when multiple\n\tcategories are listed","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20250606083808.1226386-1-stefan.klug@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250606083808.1226386-1-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=\"utf-8\"; format=\"flowed\"","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\" ","X-TUID":"ANHq9aqB9hca","Resent-From":"Kieran Bingham ","Resent-To":"parsemail@patchwork.libcamera.org"}},{"id":34605,"web_url":"https://patchwork.libcamera.org/comment/34605/","msgid":"<174973696957.81270.15217148005826219207@localhost>","date":"2025-06-12T14:02:49","subject":"Re: [PATCH] libcamera: base: Fix log level parsing when multiple\n\tcategories are listed","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the review.\n\nQuoting Barnabás Pőcze (2025-06-06 11:11:03)\n> 2025. 06. 06. 10:37 keltezéssel, Stefan Klug írta:\n> > For a list of log levels like LIBCAMERA_LOG_LEVELS=\"CatA:0,CatB:1\" only\n> > the severity of the last entry is correctly parsed.\n> > \n> > Due to the change of level to a string_view in 24c2caa1c1b3 (\"libcamera:\n> > base: log: Use `std::string_view` to avoid some copies\") the level is no\n> > longer necessarily null terminated as it is a view on the original data.\n> > \n> > Replace the check for a terminating null by a check for the end position\n> > to fix the issue.\n> \n> Oops, sorry. I hope it wasn't too hard to find.\n\nNo worries, it just took a while to realize that I'm missing some\nmessages :-)\n\n> \n> \n> > \n> > Fixes: 24c2caa1c1b3 (\"libcamera: base: log: Use `std::string_view` to avoid some copies\")\n> > Signed-off-by: Stefan Klug \n> > ---\n> > src/libcamera/base/log.cpp | 5 +++--\n> > 1 file changed, 3 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> > index 8bf3e1daa9c6..e024b58d7180 100644\n> > --- a/src/libcamera/base/log.cpp\n> > +++ b/src/libcamera/base/log.cpp\n> > @@ -690,8 +690,9 @@ LogSeverity Logger::parseLogLevel(std::string_view level)\n> > unsigned int severity = LogInvalid;\n> > \n> > if (std::isdigit(level[0])) {\n> > - auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);\n> > - if (ec != std::errc() || *end != '\\0' || severity > LogFatal)\n> > + const char* levelEnd = level.data() + level.size();\n> \n> const char *levelEnd ?\n\nRight, fixed that.\n\n> \n> Could a test be added? Maybe something like this:\n> \n> ---\n> diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp\n> index 0b999738d..8587ea30d 100644\n> --- a/test/log/log_api.cpp\n> +++ b/test/log/log_api.cpp\n> @@ -26,10 +26,42 @@ using namespace std;\n> using namespace libcamera;\n> \n> LOG_DEFINE_CATEGORY(LogAPITest)\n> +LOG_DEFINE_CATEGORY(Cat0)\n> +LOG_DEFINE_CATEGORY(Cat1)\n> +LOG_DEFINE_CATEGORY(Cat2)\n> +LOG_DEFINE_CATEGORY(Cat3)\n> +LOG_DEFINE_CATEGORY(Cat4)\n> \n> class LogAPITest : public Test\n> {\n> protected:\n> + int testEnvLevels()\n> + {\n> + setenv(\"LIBCAMERA_LOG_LEVELS\",\n> + \"Cat0:0,Cat0:9999,Cat1:INFO,Cat1:INVALID,Cat2:2,Cat2:-1,Cat3:ERROR,Cat3:{[]},Cat4:4,Cat4:rubbish\",\n> + true);\n> + logSetTarget(libcamera::LoggingTargetNone);\n> +\n> + const std::pair expected[] = {\n> + { _LOG_CATEGORY(Cat0)(), libcamera::LogDebug },\n> + { _LOG_CATEGORY(Cat1)(), libcamera::LogInfo },\n> + { _LOG_CATEGORY(Cat2)(), libcamera::LogWarning },\n> + { _LOG_CATEGORY(Cat3)(), libcamera::LogError },\n> + { _LOG_CATEGORY(Cat4)(), libcamera::LogFatal },\n> + };\n> + bool ok = true;\n> +\n> + for (const auto &[c, s] : expected) {\n> + if (c.severity() != s) {\n> + ok = false;\n> + cerr << \"Severity of \" << c.name() << \" (\" << c.severity() << \") \"\n> + << \"does not equal \" << s << endl;\n> + }\n> + }\n> +\n> + return ok ? TestPass : TestFail;\n> + }\n> +\n> void doLogging()\n> {\n> logSetLevel(\"LogAPITest\", \"DEBUG\");\n> @@ -135,7 +167,11 @@ protected:\n> \n> int run() override\n> {\n> - int ret = testFile();\n> + int ret = testEnvLevels();\n> + if (ret != TestPass)\n> + return TestFail;\n> +\n> + ret = testFile();\n> if (ret != TestPass)\n> return TestFail;\n\nThanks for writing this. I added it as a patch to v2. I hope you are\nfine with being tagged as co-author.\n\nBest regards,\nStefan\n\n> \n> ---\n> \n> Reviewed-by: Barnabás Pőcze \n> \n> \n> > + auto [end, ec] = std::from_chars(level.data(), levelEnd, severity);\n> > + if (ec != std::errc() || end != levelEnd || severity > LogFatal)\n> > severity = LogInvalid;\n> > } else {\n> > for (unsigned int i = 0; i < std::size(names); ++i) {\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":["parsemail@patchwork.libcamera.org","kbingham@ideasonboard.com"],"Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0C2A1BDE6B\n\tfor ;\n\tMon, 23 Jun 2025 14:11:57 +0000 (UTC)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net\n\t[86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 57FBD15BF\n\tfor ;\n\tMon, 23 Jun 2025 16:11:40 +0200 (CEST)","from perceval.ideasonboard.com\n\tby perceval.ideasonboard.com with LMTP id cHzmBAreSmjkNC0A4E0KoQ\n\t(envelope-from )\n\tfor ; Thu, 12 Jun 2025 16:02:50 +0200","from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\tby perceval.ideasonboard.com (Postfix) with ESMTPS\n\tid 791C6928;\tThu, 12 Jun 2025 16:02:49 +0200 (CEST)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 457DC68DBF;\n\tThu, 12 Jun 2025 16:02:56 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5DEA66154C\n\tfor ;\n\tThu, 12 Jun 2025 16:02:53 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:3176:2bf8:8f04:4c4])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 66F7A7E4;\n\tThu, 12 Jun 2025 16:02:44 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750687900;\n\tbh=F4oLhhrtTu4SWBX55mvi170FdonlgIQX8lDXwZ4KNzQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:List-Id:\n\tList-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe:\n\tResent-From:Resent-To:From;\n\tb=bMHK5nSVlwoxLYw5j5oZNrA4GJ2VbuVPjzLup8815OsGeQZGSFjwd/g06Sp0aMBqr\n\tRVBjtUa9YZYMJbyWYhFSlXVR6CEZKK74L0OTjzCs1CVGj48YTVq9hWI0VJafqlLV7N\n\tVYI+N07W2YLSwcFm8cGCMngX6N8/SNDqEf1acbYE=","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1749736964;\n\tbh=F4oLhhrtTu4SWBX55mvi170FdonlgIQX8lDXwZ4KNzQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=CXBcEA7sFUBi+9EWygIPS/ZRF8kCHTXBGEN9XV5vE1vpcvlp8baGJEsybftaeJela\n\tzt2ugdNcRoow1rknOQzGQKe9v43564s/OETc2KZcDx01Y4FwhE0QHMX/0lYJCY8w6g\n\tJJIdHODDpqjpl/YKZuZkFqXfchRU+kfX9ntg/Irc="],"Authentication-Results":["perceval.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.a=rsa-sha256 header.s=mail header.b=CXBcEA7s; dkim-atps=neutral","lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"CXBcEA7s\";\tdkim-atps=neutral"],"Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250606083808.1226386-1-stefan.klug@ideasonboard.com>\n\t","Subject":"Re: [PATCH] libcamera: base: Fix log level parsing when multiple\n\tcategories are listed","From":"Stefan Klug ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 12 Jun 2025 16:02:49 +0200","Message-ID":"<174973696957.81270.15217148005826219207@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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\" ","X-TUID":"nb2TMEZwzDck","Resent-From":"Kieran Bingham ","Resent-To":"parsemail@patchwork.libcamera.org"}}]