[{"id":1837,"web_url":"https://patchwork.libcamera.org/comment/1837/","msgid":"<20190611101710.GC5016@pendragon.ideasonboard.com>","date":"2019-06-11T10:17:10","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa_manager: Fix\n\thandling of 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 11:00:23PM +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 operating directly on the returned char array instead of\n> turning it into a std::string.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/ipa_manager.cpp | 23 +++++++++++++----------\n> 1 file changed, 13 insertions(+), 10 deletions(-)\n> \n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index f689aa69b7284092..b9b772d1d5e7795f 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\nSmall improvement, to avoid constructing an empty string,\n\n\t\tif (count) {\n\t\t\tstd::string path(modulePaths, count);\n\t\t\taddDir(path.c_str());\n\t\t}\n\nCould you test this with a LIBCAMERA_IPA_MODULE_PATH that contains \"::\"\nand that ends with \":\" ?\n\nReviewed-by: Laurent Pinchart \n\n> +\n> +\t\tif (*delim == '\\0')\n> +\t\t\tbreak;\n> +\n> +\t\tmodulePaths += count + 1;\n> \t}\n> }\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 87ECB62FCC\n\tfor ;\n\tTue, 11 Jun 2019 12:17:26 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 05406320;\n\tTue, 11 Jun 2019 12:17:25 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1560248246;\n\tbh=MX6P1Lk52gSditYNbiCb5TORPgkqWUaQqFVJrKEZEdY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GqvCAyRiALIIKIG3aYU5v5j/jKbP8WBAERpDbxMOMdflt3I5lSpLWPQDgHqUc/SMw\n\tRemE5QRmujv8XGqDDjBlWin251WyHL6Yl3LthldMUEU6H6OlRSXkd7rvtwMw40bvpB\n\tBBWojROG9XWRg3rAVfENw4+jc4T2jqt0H7LB8aw4=","Date":"Tue, 11 Jun 2019 13:17:10 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190611101710.GC5016@pendragon.ideasonboard.com>","References":"<20190610210023.4185-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":"<20190610210023.4185-1-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa_manager: Fix\n\thandling of 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":"Tue, 11 Jun 2019 10:17:26 -0000"}},{"id":1851,"web_url":"https://patchwork.libcamera.org/comment/1851/","msgid":"<20190611140242.GB25229@bigcity.dyn.berto.se>","date":"2019-06-11T14:02:42","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa_manager: Fix\n\thandling of unset LIBCAMERA_IPA_MODULE_PATH","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2019-06-11 13:17:10 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Mon, Jun 10, 2019 at 11:00:23PM +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 operating directly on the returned char array instead of\n> > turning it into a std::string.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/libcamera/ipa_manager.cpp | 23 +++++++++++++----------\n> > 1 file changed, 13 insertions(+), 10 deletions(-)\n> > \n> > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> > index f689aa69b7284092..b9b772d1d5e7795f 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> Small improvement, to avoid constructing an empty string,\n> \n> \t\tif (count) {\n> \t\t\tstd::string path(modulePaths, count);\n> \t\t\taddDir(path.c_str());\n> \t\t}\n> \n> Could you test this with a LIBCAMERA_IPA_MODULE_PATH that contains \"::\"\n> and that ends with \":\" ?\n\nIt works.\n\n> \n> Reviewed-by: Laurent Pinchart \n\nThanks, pushed to master with your tag.\n\n> \n> > +\n> > +\t\tif (*delim == '\\0')\n> > +\t\t\tbreak;\n> > +\n> > +\t\tmodulePaths += count + 1;\n> > \t}\n> > }\n> > \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5C37761E1B\n\tfor ;\n\tTue, 11 Jun 2019 16:02:44 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id h10so5882160ljg.0\n\tfor ;\n\tTue, 11 Jun 2019 07:02:44 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tc5sm2558144lfm.7.2019.06.11.07.02.42\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tTue, 11 Jun 2019 07:02:42 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=iW1lY+3/tbF3fhO5yCmSYoFabkAXFQTgc5AO+J7wu2M=;\n\tb=gihS3CHUgQfeQ2Eok18esBFW8nmty6KVKuOODIaML1J6nNxfuWLr9rziMrY7XXAZxy\n\tNOICEQ1GzJVsk8VRXSWn3psNJxgs0xHv/wGwtttUfN5CI0+jjZnIjZySzEeahO7M2FEw\n\te3qqDVyhRk1KsAg2s2fWqlTXc/JEKy4C+YX11MW5kzDT6YroavnNpo1hbnaQPIHMpTgj\n\tFM+grlxN1QXzmkSNBlVH+amgkaTxxUxt/hJ5bGkIxigkD9SHIxRfAqGIV27IqABX2Rmb\n\tJQ6K9Pf5YjR8m3l6uc+XhFDaqVOm8fLXRzFhPh0BhxSf8bKX4E/PSUYpZmkJ/wFvbBcX\n\tQ9HA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=iW1lY+3/tbF3fhO5yCmSYoFabkAXFQTgc5AO+J7wu2M=;\n\tb=rsErHx+yw1NZgRTgXDy1N3MBH9ueSiauTBIcanvKhL6yZWw8k39Jn61OX+b7LBdTpG\n\tk29P9n3kkIr2+d5iVpRMdp8cljGmxz2Qiv5TKMzzeLMLhd6LP6eppX+Qlo9yWh+c1LGp\n\tXQeLNMd3xBuMS78OoHyi2rMiAzyWaI54VgWjSo0pDDNirCgKOZ5Utve03hZ2Ospd+kHN\n\t/g6zIzqYC1/4PQsur4GbwyW91J5DKfr7Izeiybh4FSHglgVcZeL90w68TgEQhlt0GrlR\n\teeXaEsg9P+38RveET/7Yb99NLp380NOwY+85jloUW4nSgU+TAl3bc8OPoZCta7XeaIGl\n\tjhIQ==","X-Gm-Message-State":"APjAAAWLr92rV71W4wWI6UINsUyb14D3upL5Hw9B4/DZ8TjRwvCzm10x\n\tBR6R1adwc1tqvXLqdGyyBinfuJU6LMY=","X-Google-Smtp-Source":"APXvYqyxdB6Cs9mE9YyOFaH11yBCuUCkQPWIYTNxFbLWPSdIUs3PWUPldKMhZZSvQI5wa33+db9iEw==","X-Received":"by 2002:a2e:2b8d:: with SMTP id\n\tr13mr28834419ljr.145.1560261763436; \n\tTue, 11 Jun 2019 07:02:43 -0700 (PDT)","Date":"Tue, 11 Jun 2019 16:02:42 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190611140242.GB25229@bigcity.dyn.berto.se>","References":"<20190610210023.4185-1-niklas.soderlund@ragnatech.se>\n\t<20190611101710.GC5016@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190611101710.GC5016@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.0 (2019-05-25)","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa_manager: Fix\n\thandling of 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":"Tue, 11 Jun 2019 14:02:44 -0000"}}]