[{"id":1831,"web_url":"https://patchwork.libcamera.org/comment/1831/","msgid":"<20190610151217.GB11078@pendragon.ideasonboard.com>","date":"2019-06-10T15:12:17","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa_manager: Fix handling\n\tof unset LIBCAMERA_IPA_MODULE_PATH","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Mon, Jun 10, 2019 at 03:45:31PM +0200, Niklas Söderlund wrote:\n> If the environment variable LIBCAMERA_IPA_MODULE_PATH is not set\n> utils::secure_getenv() will return a nullptr. Assigning a nullptr to a\n> std::string is not valid and results in a crash,\n> \n> terminate called after throwing an instance of 'std::logic_error'\n> what(): basic_string::_M_construct null not valid\n> \n> Fix this by first checking if the char array return from\n> utils::secure_getenv() is empty before creating a std::string from it.\n\nOops :-S\n\nstd::string::string():\n\n\"Constructs the string with the contents initialized with a copy of the\nnull-terminated character string pointed to by s. The length of the\nstring is determined by the first null character. The behavior is\nundefined if [s, s + Traits::length(s)) is not a valid range (for\nexample, if s is a null pointer).\"\n\nShame they didn't specify that passing a nullptr would construct an\nempty string.\n\n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/ipa_manager.cpp | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index f689aa69b7284092..7ea7a8cbff5f15a0 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -34,10 +34,11 @@ IPAManager::IPAManager()\n> {\n> \taddDir(IPA_MODULE_DIR);\n> \n> -\tstd::string modulePaths = utils::secure_getenv(\"LIBCAMERA_IPA_MODULE_PATH\");\n> -\tif (modulePaths.empty())\n> +\tconst char *envModulePaths = utils::secure_getenv(\"LIBCAMERA_IPA_MODULE_PATH\");\n> +\tif (!envModulePaths)\n> \t\treturn;\n> \n> +\tstd::string modulePaths = envModulePaths;\n> \tfor (size_t pos = 0, delim = 0; delim != std::string::npos;\n> \t pos = delim + 1) {\n> \t\tdelim = modulePaths.find(':', pos);\n\nIf we want to avoid the conversion to an std::string, we could also try\nthe (untested) following code. Up to you.\n\ndiff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\nindex f689aa69b728..b9b772d1d5e7 100644\n--- a/src/libcamera/ipa_manager.cpp\n+++ b/src/libcamera/ipa_manager.cpp\n@@ -34,19 +34,22 @@ IPAManager::IPAManager()\n {\n \taddDir(IPA_MODULE_DIR);\n\n-\tstd::string modulePaths = utils::secure_getenv(\"LIBCAMERA_IPA_MODULE_PATH\");\n-\tif (modulePaths.empty())\n+\tconst char *modulePaths = utils::secure_getenv(\"LIBCAMERA_IPA_MODULE_PATH\");\n+\tif (!modulePaths)\n \t\treturn;\n\n-\tfor (size_t pos = 0, delim = 0; delim != std::string::npos;\n-\t pos = delim + 1) {\n-\t\tdelim = modulePaths.find(':', pos);\n-\t\tsize_t count = delim == std::string::npos ? delim : delim - pos;\n-\t\tstd::string path = modulePaths.substr(pos, count);\n-\t\tif (path.empty())\n-\t\t\tcontinue;\n+\twhile (1) {\n+\t\tconst char *delim = strchrnul(modulePaths, ':');\n+\t\tsize_t count = delim - modulePaths;\n\n-\t\taddDir(path.c_str());\n+\t\tstd::string path(modulePaths, count);\n+\t\tif (!path.empty())\n+\t\t\taddDir(path.c_str());\n+\n+\t\tif (*delim == '\\0')\n+\t\t\tbreak;\n+\n+\t\tmodulePaths += count + 1;\n \t}\n }","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 03A8A635EC\n\tfor ;\n\tMon, 10 Jun 2019 17:12:32 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 573A79CB;\n\tMon, 10 Jun 2019 17:12:32 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1560179552;\n\tbh=ybb+uRk51nXfNBB2cYouo4CbjjAo2K4jB7hGjxHM7NI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=X7RFUXmVjBdyKmvLgtNsyINY2mPaTRft9gVelD5FayP2o8TPDDIitb2QVIurwFgTS\n\ts/gwsAIspEeP0pk0R850gSlAsq0xsR1WiaK8REUHeXCNis+8edVJkyu3V6vunV43qF\n\tsiM98SgkkrE/KxsbQK3Y2chXwvQHmU2pdHt06TzI=","Date":"Mon, 10 Jun 2019 18:12:17 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190610151217.GB11078@pendragon.ideasonboard.com>","References":"<20190610134531.17316-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190610134531.17316-1-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa_manager: Fix handling\n\tof unset LIBCAMERA_IPA_MODULE_PATH","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 10 Jun 2019 15:12:33 -0000"}}]