[{"id":27044,"web_url":"https://patchwork.libcamera.org/comment/27044/","msgid":"","date":"2023-05-04T09:40:59","subject":"Re: [libcamera-devel] [PATCH 02/13] libcamera: ipa: Remove\n\tcharacter restriction on the IPA name","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Wed, May 03, 2023 at 01:20:24PM +0100, Naushir Patuck via libcamera-devel wrote:\n> Remove the restriction on using the '/' character in the IPA name\n> string. This allows more flexibility in IPA directory structures where\n> different IPA modules might live in subdirectories under the usual\n> src/ipa// top level directory.\n>\n> Signed-off-by: Naushir Patuck \n\nLooks good!\nReviewed-by: Jacopo Mondi \n\n> ---\n> src/libcamera/ipa_module.cpp | 11 +++++------\n> 1 file changed, 5 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> index c152153c180f..3ca7074b3b58 100644\n> --- a/src/libcamera/ipa_module.cpp\n> +++ b/src/libcamera/ipa_module.cpp\n> @@ -225,9 +225,9 @@ Span elfLoadSymbol(Span elf, const char *symbol)\n> * \\brief The name of the IPA module\n> *\n> * The name may be used to build file system paths to IPA-specific resources.\n> - * It shall only contain printable characters, and may not contain '/', '*',\n> - * '?' or '\\'. For IPA modules included in libcamera, it shall match the\n> - * directory of the IPA module in the source tree.\n> + * It shall only contain printable characters, and may not contain '*', '?' or\n> + * '\\'. For IPA modules included in libcamera, it shall match the directory of\n> + * the IPA module in the source tree.\n> *\n> * \\todo Allow user to choose to isolate open source IPAs\n> */\n> @@ -304,9 +304,8 @@ int IPAModule::loadIPAModuleInfo()\n> \tstd::string ipaName = info_.name;\n> \tauto iter = std::find_if_not(ipaName.begin(), ipaName.end(),\n> \t\t\t\t [](unsigned char c) -> bool {\n> -\t\t\t\t\t return isprint(c) && c != '/' &&\n> -\t\t\t\t\t\t c != '?' && c != '*' &&\n> -\t\t\t\t\t\t c != '\\\\';\n> +\t\t\t\t\t return isprint(c) && c != '?' &&\n> +\t\t\t\t\t\t c != '*' && c != '\\\\';\n> \t\t\t\t });\n> \tif (iter != ipaName.end()) {\n> \t\tLOG(IPAModule, Error)\n> --\n> 2.34.1\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 6168BC0DA4\n\tfor ;\n\tThu, 4 May 2023 09:41:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BA8D961EAC;\n\tThu, 4 May 2023 11:41:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C0B0961EAC\n\tfor ;\n\tThu, 4 May 2023 11:41:02 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 63DA2755;\n\tThu, 4 May 2023 11:40:59 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683193264;\n\tbh=7UhQj5YX0ehUIVJCcUe4126yYb3MBcAjA3ftxae4jMA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=rVlENdhkD8nzR2HiZq6gchDh+Q/m4x0w9bElEJ8Jg3Ed6lXJySVzaSu6JsXqu4a1q\n\tTsxmtO/wpy+ijpXzPfZXgJe1vFwN/ExSvH8DfEF2UsDUIbYIvl/u5BfkO3km86LEsN\n\tzzv56lMw0kwUp3XmwFLfJEXh+CbET+MhWBxp90CYcOhSjoJwjBYdIFbdpo3B4/chTG\n\t21cNLKGwLnierbeRNoZ6jBTdZA8DaZQKjvIS6dP/U0zd7WK+MSRFGmf5jANvt13Cnq\n\t3+DgoJYjczVVLfFghneskzc5+YPyEdXmjTb7LxPoiqnBu8kjQqaNQh2y0lyDkxyWsW\n\tmOq3JhgOpoe8g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683193259;\n\tbh=7UhQj5YX0ehUIVJCcUe4126yYb3MBcAjA3ftxae4jMA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HWqvNzUWt2iisX0d435VDDNknWWReIHrdrh79miIaa1OkzlkDr0HZ58QWBOEDVgMs\n\tgkc1FDDlXjZh8PBSwYzDhLEwTcUETJ9pSTQKeeDnGZoe+7zKtvNmTJXDRcr31V1yjf\n\t8+Xe4waXz7x6G+Nphe2ENXIDmlht3OTNM0gIAGYk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HWqvNzUW\"; dkim-atps=neutral","Date":"Thu, 4 May 2023 11:40:59 +0200","To":"Naushir Patuck ","Message-ID":"","References":"<20230503122035.32026-1-naush@raspberrypi.com>\n\t<20230503122035.32026-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230503122035.32026-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 02/13] libcamera: ipa: Remove\n\tcharacter restriction on the IPA name","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27047,"web_url":"https://patchwork.libcamera.org/comment/27047/","msgid":"<20230504100447.GG4551@pendragon.ideasonboard.com>","date":"2023-05-04T10:04:47","subject":"Re: [libcamera-devel] [PATCH 02/13] libcamera: ipa: Remove\n\tcharacter restriction on the IPA name","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Wed, May 03, 2023 at 01:20:24PM +0100, Naushir Patuck via libcamera-devel wrote:\n> Remove the restriction on using the '/' character in the IPA name\n> string. This allows more flexibility in IPA directory structures where\n> different IPA modules might live in subdirectories under the usual\n> src/ipa// top level directory.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> src/libcamera/ipa_module.cpp | 11 +++++------\n> 1 file changed, 5 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> index c152153c180f..3ca7074b3b58 100644\n> --- a/src/libcamera/ipa_module.cpp\n> +++ b/src/libcamera/ipa_module.cpp\n> @@ -225,9 +225,9 @@ Span elfLoadSymbol(Span elf, const char *symbol)\n> * \\brief The name of the IPA module\n> *\n> * The name may be used to build file system paths to IPA-specific resources.\n> - * It shall only contain printable characters, and may not contain '/', '*',\n> - * '?' or '\\'. For IPA modules included in libcamera, it shall match the\n> - * directory of the IPA module in the source tree.\n> + * It shall only contain printable characters, and may not contain '*', '?' or\n> + * '\\'. For IPA modules included in libcamera, it shall match the directory of\n> + * the IPA module in the source tree.\n> *\n> * \\todo Allow user to choose to isolate open source IPAs\n> */\n> @@ -304,9 +304,8 @@ int IPAModule::loadIPAModuleInfo()\n> \tstd::string ipaName = info_.name;\n> \tauto iter = std::find_if_not(ipaName.begin(), ipaName.end(),\n> \t\t\t\t [](unsigned char c) -> bool {\n> -\t\t\t\t\t return isprint(c) && c != '/' &&\n> -\t\t\t\t\t\t c != '?' && c != '*' &&\n> -\t\t\t\t\t\t c != '\\\\';\n> +\t\t\t\t\t return isprint(c) && c != '?' &&\n> +\t\t\t\t\t\t c != '*' && c != '\\\\';\n\nI think I mentioned in the review of the previous version that we should\ndisallow \"..\" to avoid escaping into the file system. I however have a\nbit of a hard time at this point to decide if it would be best\nimplemented here or elsewhere, or what the exact naming rules should be.\nI'll thus add a\n\n * \\todo Consider module naming restrictions to avoid escaping from a base\n * directory. Forbidding \"..\" may be enough, but this may be best implemented\n * in a different layer.\n\ncomment.\n\nReviewed-by: Laurent Pinchart \n\n> \t\t\t\t });\n> \tif (iter != ipaName.end()) {\n> \t\tLOG(IPAModule, Error)","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 E6380C0DA4\n\tfor ;\n\tThu, 4 May 2023 10:04:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 525E7627DE;\n\tThu, 4 May 2023 12:04:37 +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 A646561EAE\n\tfor ;\n\tThu, 4 May 2023 12:04:35 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(133-32-181-51.west.xps.vectant.ne.jp [133.32.181.51])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 89FBC6E0;\n\tThu, 4 May 2023 12:04:30 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683194677;\n\tbh=93dfcmEbT/DRGMuOq5aaUpWpu0slXf8cJ+W2a3AG5IM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=S5cq6/X8mj24/JYXqREvM1/nwqS83UfA+CSbWtinvuwhgql4IxZ+eshBUB5kMJ8AL\n\traOOURNnfO7KABkhknwgZtgY7l+cPxz9ENrGfx+jq7Md7KuZrDJZV4DwRc88GNIGVW\n\t0EQyr6Kzo7udTepmmxUK/0NS35hXP635cS6tF+zJbEPKibWa02lvh2SfyS2XKcL3+h\n\tOEBSoat3KMA5f3nqXQFZfSmoIiGOSiH9uwteEkwcqGV2YStweyoWAEj2pNTWpWZkwe\n\tcD+kP0GY0E+J+HzlCiDnvoaJRrtsl+NfUUtN3TFXbvENCEAW6iiJaPjYYwFivrkODl\n\th94gq8fbIRxEg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683194672;\n\tbh=93dfcmEbT/DRGMuOq5aaUpWpu0slXf8cJ+W2a3AG5IM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PHl6v87o6FM3qL1jAXpvY4vM/iRWm4bqFKhOI8nP+jRHw0kjcSNpLAd6XzAZKaB3t\n\tDE2brX1iDvgJ/TLqCpHj7k75JxEOTXidTfGR3ZLqAPbwMg2RtLuUqb8jGFg2P+MKzq\n\tJBkKq4Bknt7m543of4BQnR14LsyYac2+VNnbfGEw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"PHl6v87o\"; dkim-atps=neutral","Date":"Thu, 4 May 2023 13:04:47 +0300","To":"Naushir Patuck ","Message-ID":"<20230504100447.GG4551@pendragon.ideasonboard.com>","References":"<20230503122035.32026-1-naush@raspberrypi.com>\n\t<20230503122035.32026-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230503122035.32026-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 02/13] libcamera: ipa: Remove\n\tcharacter restriction on the IPA name","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]