[{"id":26175,"web_url":"https://patchwork.libcamera.org/comment/26175/","msgid":"","date":"2022-12-30T19:52:05","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: yaml_parser: Use C\n\tlocale","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Fri, Dec 30, 2022 at 12:23:09AM +0000, Kieran Bingham wrote:\n> When parsing configuration files on systems with differing locales, the\n> use of strtod can produce different results, or in the worst case - fail\n> to parse expected values.\n\nThere's something missing here:\n\nFix this by using strtod_l() instead. To avoid constructing and\ndestructing a locale_t instance for every too to strtod_l(), create an\nRAII class that wraps the locale_t and use it to provide a global \"C\"\nlocale.\n\n> Provide an RAII implementation to construct a locale specific to the\n> expected mappings for configuration files provided by libcamera.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=174\n> Bug: https://github.com/raspberrypi/libcamera/issues/29\n> Reported-by: https://github.com/kralo\n> Reported-by: Hannes Winkler \n> Reviewed-by: Laurent Pinchart \n> Signed-off-by: Kieran Bingham \n> \n> ---\n> v2:\n> - use const char * over std::string\n> - use C++ static casts\n> - fix blank lines\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> src/libcamera/yaml_parser.cpp | 34 +++++++++++++++++++++++++++++++++-\n> 1 file changed, 33 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> index d8a7c2f9250f..c7fd80cdea1b 100644\n> --- a/src/libcamera/yaml_parser.cpp\n> +++ b/src/libcamera/yaml_parser.cpp\n> @@ -31,6 +31,38 @@ namespace {\n> /* Empty static YamlObject as a safe result for invalid operations */\n> static const YamlObject empty;\n> \n> +/*\n> + * Construct a global RAII locale for use by all YAML parser instances to\n> + * ensure consistency when parsing configuration files and types regardless of\n> + * the host Locale configuration.\n\ns/host/system/\ns/Locale/locale/\n\n> + *\n> + * For more information see:\n> + * - https://bugs.libcamera.org/show_bug.cgi?id=174\n> + */\n> +class Locale\n> +{\n> +public:\n> +\tLocale(const char *locale)\n> +\t{\n> +\t\tlocale_ = newlocale(LC_ALL_MASK, locale, static_cast(0));\n> +\t\tif (locale_ == static_cast(0))\n> +\t\t\tLOG(YamlParser, Fatal)\n> +\t\t\t\t<< \"Failed to construct a locale\";\n> +\t}\n> +\n> +\t~Locale()\n> +\t{\n> +\t\tfreelocale(locale_);\n> +\t}\n> +\n> +\tlocale_t locale() { return locale_; }\n> +\n> +private:\n> +\tlocale_t locale_;\n> +};\n> +\n> +static Locale yamlLocale(\"C\");\n\nYou can drop static as you're in an anonymous namespace.\n\nAll these issues are minor, no need to post a v3 just for this.\n\n> +\n> } /* namespace */\n> \n> /**\n> @@ -283,7 +315,7 @@ std::optional YamlObject::get() const\n> \tchar *end;\n> \n> \terrno = 0;\n> -\tdouble value = std::strtod(value_.c_str(), &end);\n> +\tdouble value = strtod_l(value_.c_str(), &end, yamlLocale.locale());\n> \n> \tif ('\\0' != *end || errno == ERANGE)\n> \t\treturn std::nullopt;","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 F26FBC3220\n\tfor ;\n\tFri, 30 Dec 2022 19:52:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 36DDD625CF;\n\tFri, 30 Dec 2022 20:52:12 +0100 (CET)","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 B087461F11\n\tfor ;\n\tFri, 30 Dec 2022 20:52:10 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 08C8D2F5;\n\tFri, 30 Dec 2022 20:52:09 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1672429932;\n\tbh=/aIr5aIAFBinG8a+8wk9FCZ3b3b4XmlVRUtAaNgaVig=;\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=qaWO8p4uMeMh1rVroBh35UlXfkuqqQduOnyXNO2DvvSLrNk4f1kuna5jQfC5wGJiD\n\tOgrBzxk1LSHGwxAsfR3H6dCA/rbet3FbIUY+q+q/nHoNWPBoBXR3HgrlGEjR8cIXgZ\n\tLcT3xJLQDg6J+aP7C3a7r2Nejdyq5RmRjbFHOgj8CMpic5gODF13lCDPysVxr6eVO+\n\tnwmJ4WD0Q3HWrVaiAPBQegARjRv4SkdSmmDYiH1Ucwhl8KeSH5B005yu048AgOqZXT\n\tHsC+XyjdYprPFvba6iItTx7Cfh5SveQT0USTEBR8P9lLy8tqhEokgwyZv4Q5tpF9Kj\n\teqlIEALoaPctQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1672429930;\n\tbh=/aIr5aIAFBinG8a+8wk9FCZ3b3b4XmlVRUtAaNgaVig=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jpgoPBdXjWe2kZIgjpnDFgmr1C0h3qTeYR52XD2RJujcTpTZyoDZWqb1HN5Lw1F+f\n\tP/Gg+9MZGUkMG0kFdgk7w3TEsxcOtBaP0/+Df1oa4uA7Ka+0bk87VCbGyZDbFGtgA/\n\tUFE1PKl0yMDilMUubAbTYcGVBQ5DnG/eJHjzk9ak="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"jpgoPBdX\"; dkim-atps=neutral","Date":"Fri, 30 Dec 2022 21:52:05 +0200","To":"Kieran Bingham ","Message-ID":"","References":"<20221230002309.3135078-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221230002309.3135078-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: yaml_parser: Use C\n\tlocale","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 ,\n\tHannes Winkler ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]