[{"id":32526,"web_url":"https://patchwork.libcamera.org/comment/32526/","msgid":"","date":"2024-12-04T17:45:01","subject":"Re: [PATCH v5 09/15] config: Look up IPA configurables in\n\tconfiguration file","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. október 1., kedd 12:28 keltezéssel, Milan Zamazal írta:\n\n> The configuration snippet:\n> \n> configuration:\n> ipa:\n> config_paths: CONFIG:PATHS:...\n> module_paths: MODULE:PATHS:...\n> force_isolation: BOOL\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/ipa_manager.cpp | 11 ++++++++---\n> src/libcamera/ipa_proxy.cpp | 16 +++++++++++-----\n> 2 files changed, 19 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 33ae74e8f..566dc1402 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -114,8 +114,11 @@ IPAManager::IPAManager()\n> \tunsigned int ipaCount = 0;\n> \n> \t/* User-specified paths take precedence. */\n> -\tconst char *modulePaths = utils::secure_getenv(\"LIBCAMERA_IPA_MODULE_PATH\");\n> -\tif (modulePaths) {\n> +\tconst auto confModulePaths =\n> +\t\tGlobalConfiguration::envOption(\n> +\t\t\t\"LIBCAMERA_IPA_MODULE_PATH\", \"ipa.module_paths\");\n> +\tif (confModulePaths.has_value()) {\n> +\t\tconst char *modulePaths = confModulePaths.value().c_str();\n\nI think there is no need for this separate variable, i.e.\n\n auto modulePaths = ...;\n if (modulePaths) {\n for (... : utils::split(*modulePaths, \":\"))\n\n\n\n> \t\tfor (const auto &dir : utils::split(modulePaths, \":\")) {\n> \t\t\tif (dir.empty())\n> \t\t\t\tcontinue;\n> @@ -289,7 +292,9 @@ bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const\n> {\n> #if HAVE_IPA_PUBKEY\n> \tchar *force = utils::secure_getenv(\"LIBCAMERA_IPA_FORCE_ISOLATION\");\n> -\tif (force && force[0] != '\\0') {\n> +\tif ((force && force[0] != '\\0') ||\n> +\t (!force && GlobalConfiguration::option(\"ipa.force_isolation\")\n> +\t\t\t .value_or(false))) {\n> \t\tLOG(IPAManager, Debug)\n> \t\t\t<< \"Isolation of IPA module \" << ipa->path()\n> \t\t\t<< \" forced through environment variable\";\n> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> index 85004737c..787d58019 100644\n> --- a/src/libcamera/ipa_proxy.cpp\n> +++ b/src/libcamera/ipa_proxy.cpp\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n> \n> +#include \"libcamera/internal/global_configuration.h\"\n> #include \"libcamera/internal/ipa_module.h\"\n> \n> /**\n> @@ -108,8 +109,11 @@ std::string IPAProxy::configurationFile(const std::string &name,\n> \tstd::string ipaName = ipam_->info().name;\n> \n> \t/* Check the environment variable first. */\n> -\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n> -\tif (confPaths) {\n> +\tauto confConfPaths =\n> +\t\tGlobalConfiguration::envOption(\n> +\t\t\t\"LIBCAMERA_IPA_CONFIG_PATH\", \"ipa.config_paths\");\n> +\tif (confConfPaths.has_value()) {\n> +\t\tconst char *confPaths = confConfPaths.value().c_str();\n\nSame here.\n\n\n> \t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n> \t\t\tif (dir.empty())\n> \t\t\t\tcontinue;\n> @@ -183,9 +187,11 @@ std::string IPAProxy::resolvePath(const std::string &file) const\n> \tstd::string proxyFile = \"/\" + file;\n> \n> \t/* Check env variable first. */\n> -\tconst char *execPaths = utils::secure_getenv(\"LIBCAMERA_IPA_PROXY_PATH\");\n> -\tif (execPaths) {\n> -\t\tfor (const auto &dir : utils::split(execPaths, \":\")) {\n> +\tconst auto execPaths =\n> +\t\tGlobalConfiguration::envOption(\n> +\t\t\t\"LIBCAMERA_IPA_PROXY_PATH\", \"ipa.proxy_paths\");\n> +\tif (execPaths.has_value()) {\n> +\t\tfor (const auto &dir : utils::split(execPaths.value().c_str(), \":\")) {\n\nJust pass `execPaths.value()` or `*execPaths`.\n\n\n> \t\t\tif (dir.empty())\n> \t\t\t\tcontinue;\n> \n> --\n> 2.44.1\n> \n\nI think it would also be desirable if `ipa.X_paths` could be defined as a proper\narray in the configuration file.\n\n\nRegards,\nBarnabás Pőcze","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 4D0F0BDB1C\n\tfor ;\n\tWed, 4 Dec 2024 17:45:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2272F660E9;\n\tWed, 4 Dec 2024 18:45:18 +0100 (CET)","from mail-10629.protonmail.ch (mail-10629.protonmail.ch\n\t[79.135.106.29])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D971F660E4\n\tfor ;\n\tWed, 4 Dec 2024 18:45:13 +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=\"i0ejMCuX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1733334310; x=1733593510;\n\tbh=1SoFPjgGob57p6g6QZ47K2z8MIlYoMtl1EFqO0GsQXo=;\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=i0ejMCuXknZDLNU1sGmzNTzuId6bp0xgZ7eV4TdShf4foanZYmspaGeCIwImGH5OH\n\tXdN23t3AxaOAnsmBSw3+5TN7H54Iv0+mJmQBBw+SqIVqvmsyVuhog9Si7rW6qUdh8N\n\tNGc3QW7GarAgIbXmmSy0omKTsekPNnh5ZFk/xIBoeqtrJGEWgljdO0IvR3mYAmCYRM\n\tp4sI/ASo+Ir6XrYM9XnUYOhAPnBtm8lmIJAcAXSKY4Us6IExSr7HbND6OYGUolWh/d\n\tPE/+FtyT7tDWV4W//DFHk/fEte4b0gWpvQeEJGDeY3A6U83M8WFt1H84S7VuIfDAXP\n\ttlkwCdTvR2QNg==","Date":"Wed, 04 Dec 2024 17:45:01 +0000","To":"Milan Zamazal ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ,\n\tNaushir Patuck ","Subject":"Re: [PATCH v5 09/15] config: Look up IPA configurables in\n\tconfiguration file","Message-ID":"","In-Reply-To":"<20241001102810.479285-10-mzamazal@redhat.com>","References":"<20241001102810.479285-1-mzamazal@redhat.com>\n\t<20241001102810.479285-10-mzamazal@redhat.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"b590a17085a55b4a4ba81bf9ecb8f561852a404c","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\" "}},{"id":32538,"web_url":"https://patchwork.libcamera.org/comment/32538/","msgid":"<87v7vyz6io.fsf@redhat.com>","date":"2024-12-05T11:57:51","subject":"Re: [PATCH v5 09/15] config: Look up IPA configurables in\n\tconfiguration file","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Barnabás,\n\nBarnabás Pőcze writes:\n\n> Hi\n>\n>\n> 2024. október 1., kedd 12:28 keltezéssel, Milan Zamazal írta:\n>\n>> The configuration snippet:\n>> \n>> configuration:\n>> ipa:\n>> config_paths: CONFIG:PATHS:...\n>> module_paths: MODULE:PATHS:...\n>> force_isolation: BOOL\n>> \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/libcamera/ipa_manager.cpp | 11 ++++++++---\n>> src/libcamera/ipa_proxy.cpp | 16 +++++++++++-----\n>> 2 files changed, 19 insertions(+), 8 deletions(-)\n>> \n>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n>> index 33ae74e8f..566dc1402 100644\n>> --- a/src/libcamera/ipa_manager.cpp\n>> +++ b/src/libcamera/ipa_manager.cpp\n>> @@ -114,8 +114,11 @@ IPAManager::IPAManager()\n>> \tunsigned int ipaCount = 0;\n>> \n>> \t/* User-specified paths take precedence. */\n>> -\tconst char *modulePaths = utils::secure_getenv(\"LIBCAMERA_IPA_MODULE_PATH\");\n>> -\tif (modulePaths) {\n>> +\tconst auto confModulePaths =\n>> +\t\tGlobalConfiguration::envOption(\n>> +\t\t\t\"LIBCAMERA_IPA_MODULE_PATH\", \"ipa.module_paths\");\n>> +\tif (confModulePaths.has_value()) {\n>> +\t\tconst char *modulePaths = confModulePaths.value().c_str();\n>\n> I think there is no need for this separate variable, i.e.\n>\n> auto modulePaths = ...;\n> if (modulePaths) {\n> for (... : utils::split(*modulePaths, \":\"))\n\nOK.\n\n>> \t\tfor (const auto &dir : utils::split(modulePaths, \":\")) {\n>> \t\t\tif (dir.empty())\n>> \t\t\t\tcontinue;\n>> @@ -289,7 +292,9 @@ bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const\n>> {\n>> #if HAVE_IPA_PUBKEY\n>> \tchar *force = utils::secure_getenv(\"LIBCAMERA_IPA_FORCE_ISOLATION\");\n>> -\tif (force && force[0] != '\\0') {\n>> +\tif ((force && force[0] != '\\0') ||\n>> +\t (!force && GlobalConfiguration::option(\"ipa.force_isolation\")\n>> +\t\t\t .value_or(false))) {\n>> \t\tLOG(IPAManager, Debug)\n>> \t\t\t<< \"Isolation of IPA module \" << ipa->path()\n>> \t\t\t<< \" forced through environment variable\";\n>> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n>> index 85004737c..787d58019 100644\n>> --- a/src/libcamera/ipa_proxy.cpp\n>> +++ b/src/libcamera/ipa_proxy.cpp\n>> @@ -14,6 +14,7 @@\n>> #include \n>> #include \n>> \n>> +#include \"libcamera/internal/global_configuration.h\"\n>> #include \"libcamera/internal/ipa_module.h\"\n>> \n>> /**\n>> @@ -108,8 +109,11 @@ std::string IPAProxy::configurationFile(const std::string &name,\n>> \tstd::string ipaName = ipam_->info().name;\n>> \n>> \t/* Check the environment variable first. */\n>> -\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n>> -\tif (confPaths) {\n>> +\tauto confConfPaths =\n>> +\t\tGlobalConfiguration::envOption(\n>> +\t\t\t\"LIBCAMERA_IPA_CONFIG_PATH\", \"ipa.config_paths\");\n>> +\tif (confConfPaths.has_value()) {\n>> +\t\tconst char *confPaths = confConfPaths.value().c_str();\n>\n> Same here.\n\nOK.\n\n>> \t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n>> \t\t\tif (dir.empty())\n>> \t\t\t\tcontinue;\n>> @@ -183,9 +187,11 @@ std::string IPAProxy::resolvePath(const std::string &file) const\n>> \tstd::string proxyFile = \"/\" + file;\n>> \n>> \t/* Check env variable first. */\n>> -\tconst char *execPaths = utils::secure_getenv(\"LIBCAMERA_IPA_PROXY_PATH\");\n>> -\tif (execPaths) {\n>> -\t\tfor (const auto &dir : utils::split(execPaths, \":\")) {\n>> +\tconst auto execPaths =\n>> +\t\tGlobalConfiguration::envOption(\n>> +\t\t\t\"LIBCAMERA_IPA_PROXY_PATH\", \"ipa.proxy_paths\");\n>> +\tif (execPaths.has_value()) {\n>> +\t\tfor (const auto &dir : utils::split(execPaths.value().c_str(), \":\")) {\n>\n> Just pass `execPaths.value()` or `*execPaths`.\n\nOK.\n\n>> \t\t\tif (dir.empty())\n>> \t\t\t\tcontinue;\n>> \n>> --\n>> 2.44.1\n>> \n>\n> I think it would also be desirable if `ipa.X_paths` could be defined as a proper\n> array in the configuration file.\n\nYes, I'll try to arrange it this way.\n\n> Regards,\n> Barnabás Pőcze","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 83972C323E\n\tfor ;\n\tThu, 5 Dec 2024 11:57:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B1F9660F3;\n\tThu, 5 Dec 2024 12:57:58 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A2D756608C\n\tfor ;\n\tThu, 5 Dec 2024 12:57:56 +0100 (CET)","from mail-ej1-f70.google.com (mail-ej1-f70.google.com\n\t[209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-48-DnAEpp-sNvK9dkY_MDnCNA-1; Thu, 05 Dec 2024 06:57:54 -0500","by mail-ej1-f70.google.com with SMTP id\n\ta640c23a62f3a-aa62e76e364so15703366b.3\n\tfor ;\n\tThu, 05 Dec 2024 03:57:54 -0800 (PST)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-aa62608d2fbsm81460766b.149.2024.12.05.03.57.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 05 Dec 2024 03:57:52 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"XnKGvekm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1733399875;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=LjgwUdkqwvhZiwHGA7fdqShej/uZ1FSGkaEupy+NQpQ=;\n\tb=XnKGvekm9Eek1zjdc9j64ZrbJmVtpspWkln3Cm7Vl2v84WrqfiZWPcebYlc/4X5tnM7zM1\n\t5YH5Z1vwcPy1dvO/jtr+zAhkv21oYkEUo1qPRTF4fqe/sOCJEIULTRP224wB/+ulSYw32v\n\tADqBhmyVOjkO063M+jJ2LTbDjuyCCiw=","X-MC-Unique":"DnAEpp-sNvK9dkY_MDnCNA-1","X-Mimecast-MFC-AGG-ID":"DnAEpp-sNvK9dkY_MDnCNA","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1733399873; x=1734004673;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=BbWkew7AXvD/waEbmt4494JtnOoG/TFcJOXCqGnvmwg=;\n\tb=YrQUTyVZznuMhnbbfjEo0g7cKGSePecBkCpnpK5Alu0bmJ9ew1Ot1faXiEJKBaMfYo\n\tMJ58WipW04ZPEYq9bv+zyyawhAVPD/CdO0b0TacSIBAyjltq8LcBwU2ztBBjDziyuNWx\n\thq7MpZVePXslxEP27UV2gILbKKdo3LTwP5jMEhRr4wsYPU1tHrNyP3Twhx3RUhGvWS9+\n\to6S16foRdPaWtQJpfyJVhUxIC725SCqjC/cu/1blD/AcI6pZeuJ/8G9eLNyDM4KAcfcH\n\tYdH+JHVUR9/C2U4vJinJJoEIKYruwkKoThyYu8H/JSQdT4rwNgRbElW0cwp5wf6JX2aI\n\tNM/g==","X-Gm-Message-State":"AOJu0Yz3hl3bVTHq+9Tl9+LPYBKckBi+xsAmh7RXzfBf/Jsz4iRQZVCC\n\tv3jQrr2KOROsaAlKqWiMmvUFo27pWCCFcAPwrYx2SdqtjzbN0GQsCNxjGIH07D13TWqPPg8Zq5Y\n\tukWt9Igng/L73qC5VmRia6VoUYm8B2A2jP+8Pl6kKdSVbT+ce+KhyRrRemt1omk9s3MRCDhg=","X-Gm-Gg":"ASbGncsf3paa1fjokyDOlVhLY6pSy+dEJLSaq3yIEPMJ502p9EFT6uE1NqUrhls0V2D\n\tq49YKTHEgu8cm7PE5L4wtvBpasqjruuJinI/zuQwdn/MibiwawG4cPJk94nXZ2381s6bsaRUWJL\n\teQ2V21okE4NPwA9xTEwiOHOTmuXDK9u2heVgzCO14AQVLN/IoY03riARvhERCHgwCn2VdjfHFox\n\tRuoWapD9MBMRdWvBzcZLwSS2JOilxV+TUHntfMM817Jn5Y6ZkC6TwePd7oxuk41EARkiV4=","X-Received":["by 2002:a17:906:32c2:b0:aa5:52db:eebd with SMTP id\n\ta640c23a62f3a-aa5f7daba8amr754656566b.26.1733399873343; \n\tThu, 05 Dec 2024 03:57:53 -0800 (PST)","by 2002:a17:906:32c2:b0:aa5:52db:eebd with SMTP id\n\ta640c23a62f3a-aa5f7daba8amr754654966b.26.1733399872916; \n\tThu, 05 Dec 2024 03:57:52 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IEZ9qLpqEMCbM75Hk9vls2ZAne+Fs+VNr8g3R2ZlbOCyNe4UN11mR5nLj9i2boqyXD8njiaZA==","From":"Milan Zamazal ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, Naushir Patuck ","Subject":"Re: [PATCH v5 09/15] config: Look up IPA configurables in\n\tconfiguration file","In-Reply-To":"\n\t( =?utf-8?b?IkJhcm5hYsOhcyBQxZFjemUiJ3M=?= message of \"Wed,\n\t04 Dec 2024 17:45:01 +0000\")","References":"<20241001102810.479285-1-mzamazal@redhat.com>\n\t<20241001102810.479285-10-mzamazal@redhat.com>\n\t","Date":"Thu, 05 Dec 2024 12:57:51 +0100","Message-ID":"<87v7vyz6io.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"utrAq-ey5hWG38j6b3EQgbph6XS4WYNf-lJya-T6qlk_1733399873","X-Mimecast-Originator":"redhat.com","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\" "}}]