[{"id":11852,"web_url":"https://patchwork.libcamera.org/comment/11852/","msgid":"<20200804230034.GS6075@pendragon.ideasonboard.com>","date":"2020-08-04T23:00:34","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipu3: Fix compilation\n\tissues with gcc5","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Mon, Aug 03, 2020 at 06:28:11PM +0200, Jacopo Mondi wrote:\n> GCC5 does not provide prototypes for the math library functions defined\n> in the math.h header for the std:: namespace.\n> \n> Include the C++ header in place of as it defines\n> overloads for the std::abs and std::fmod function.\n> \n> This goes intentionally against the libcamera coding guidelines, and\n> is reported as warning by checkpatch.py.\n\nI'm considering adjusting the guidelines specifically for cmath, as the\nmath function overloads are really nice to have, and should do the right\nthing by default.\n\n> Fixes: 968ab9bad0ed (\"libcamera: ipu3: imgu: Calculate ImgU pipe configuration\")\n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Laurent Pinchart \n\nPlease push :-)\n\n> ---\n> src/libcamera/pipeline/ipu3/imgu.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> index b7593ceb3672..eb829e096561 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -7,8 +7,8 @@\n> \n> #include \"imgu.h\"\n> \n> +#include \n> #include \n> -#include \n> \n> #include \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 B4B45BD87A\n\tfor ;\n\tTue, 4 Aug 2020 23:00:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 425306054E;\n\tWed, 5 Aug 2020 01:00:48 +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 694B160392\n\tfor ;\n\tWed, 5 Aug 2020 01:00:47 +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 BEF8427B;\n\tWed, 5 Aug 2020 01:00:46 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uqsPBXAO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596582047;\n\tbh=6CqBFWwQ5vS2cTuUuNMXxmymrsxYi7QmTS30wTrA8vc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uqsPBXAO9nKlLFb2TnslEgZXqoDGma0tyw7dgS2WAku20oqWoM958X1b+Zn9Y7bW3\n\tt4jK/M++9hd5NPDZhV05vy/HUI+hrMCUef9HvOjLNeGY2mq/u9BlIvxA4FeT9bRicd\n\tgmKtxX2D8R3+s096Fzv3Spuqo1kFaaXRpkOKJY3s=","Date":"Wed, 5 Aug 2020 02:00:34 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200804230034.GS6075@pendragon.ideasonboard.com>","References":"<20200803162811.77634-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200803162811.77634-1-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipu3: Fix compilation\n\tissues with gcc5","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11862,"web_url":"https://patchwork.libcamera.org/comment/11862/","msgid":"<20200805071029.aqmszycnsguasomm@uno.localdomain>","date":"2020-08-05T07:10:29","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipu3: Fix compilation\n\tissues with gcc5","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Wed, Aug 05, 2020 at 02:00:34AM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Mon, Aug 03, 2020 at 06:28:11PM +0200, Jacopo Mondi wrote:\n> > GCC5 does not provide prototypes for the math library functions defined\n> > in the math.h header for the std:: namespace.\n> >\n> > Include the C++ header in place of as it defines\n> > overloads for the std::abs and std::fmod function.\n> >\n> > This goes intentionally against the libcamera coding guidelines, and\n> > is reported as warning by checkpatch.py.\n>\n> I'm considering adjusting the guidelines specifically for cmath, as the\n> math function overloads are really nice to have, and should do the right\n> thing by default.\n\nWhat about\n\n--- a/Documentation/coding-style.rst\n+++ b/Documentation/coding-style.rst\n@@ -198,7 +198,14 @@ std namespace, and may declare the same names in the global namespace. The C\n compatibility headers declare names in the global namespace, and may declare\n the same names in the std namespace. Usage of the C compatibility headers is\n strongly preferred. Code shall not rely on the optional declaration of names in\n-the global or std namespace.\n+the global or std namespace. An exception to this rule is represented by the\n+[cmath|math.h] header files. As the type of the argument is particularly\n+relevant for mathematical operations, the former (cmath) defines in the std\n+namespace overloads of the same function that accept different parameter types,\n+while the latter (math.h) requires the developer to pick the right function for\n+the function arguments at hand. As an example, cmath defines std::abs(int) and\n+std::abs(float) while math.h defines abs(int) and fabs(float). For this reason\n+the inclusion of is preferred over .\n\n>\n> > Fixes: 968ab9bad0ed (\"libcamera: ipu3: imgu: Calculate ImgU pipe configuration\")\n> > Signed-off-by: Jacopo Mondi \n>\n> Reviewed-by: Laurent Pinchart \n>\n> Please push :-)\n\nThanks\n j\n>\n> > ---\n> > src/libcamera/pipeline/ipu3/imgu.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > index b7593ceb3672..eb829e096561 100644\n> > --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > @@ -7,8 +7,8 @@\n> >\n> > #include \"imgu.h\"\n> >\n> > +#include \n> > #include \n> > -#include \n> >\n> > #include \n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 BBDD3BD86F\n\tfor ;\n\tWed, 5 Aug 2020 07:06:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5DBE160557;\n\tWed, 5 Aug 2020 09:06:50 +0200 (CEST)","from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6703E60552\n\tfor ;\n\tWed, 5 Aug 2020 09:06:49 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id E4785200005;\n\tWed, 5 Aug 2020 07:06:48 +0000 (UTC)"],"Date":"Wed, 5 Aug 2020 09:10:29 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200805071029.aqmszycnsguasomm@uno.localdomain>","References":"<20200803162811.77634-1-jacopo@jmondi.org>\n\t<20200804230034.GS6075@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200804230034.GS6075@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipu3: Fix compilation\n\tissues with gcc5","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11890,"web_url":"https://patchwork.libcamera.org/comment/11890/","msgid":"<20200805144525.GH6751@pendragon.ideasonboard.com>","date":"2020-08-05T14:45:25","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipu3: Fix compilation\n\tissues with gcc5","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Aug 05, 2020 at 09:10:29AM +0200, Jacopo Mondi wrote:\n> On Wed, Aug 05, 2020 at 02:00:34AM +0300, Laurent Pinchart wrote:\n> > On Mon, Aug 03, 2020 at 06:28:11PM +0200, Jacopo Mondi wrote:\n> > > GCC5 does not provide prototypes for the math library functions defined\n> > > in the math.h header for the std:: namespace.\n> > >\n> > > Include the C++ header in place of as it defines\n> > > overloads for the std::abs and std::fmod function.\n> > >\n> > > This goes intentionally against the libcamera coding guidelines, and\n> > > is reported as warning by checkpatch.py.\n> >\n> > I'm considering adjusting the guidelines specifically for cmath, as the\n> > math function overloads are really nice to have, and should do the right\n> > thing by default.\n> \n> What about\n> \n> --- a/Documentation/coding-style.rst\n> +++ b/Documentation/coding-style.rst\n> @@ -198,7 +198,14 @@ std namespace, and may declare the same names in the global namespace. The C\n> compatibility headers declare names in the global namespace, and may declare\n> the same names in the std namespace. Usage of the C compatibility headers is\n> strongly preferred. Code shall not rely on the optional declaration of names in\n> -the global or std namespace.\n> +the global or std namespace. An exception to this rule is represented by the\n> +[cmath|math.h] header files. As the type of the argument is particularly\n> +relevant for mathematical operations, the former (cmath) defines in the std\n> +namespace overloads of the same function that accept different parameter types,\n> +while the latter (math.h) requires the developer to pick the right function for\n> +the function arguments at hand. As an example, cmath defines std::abs(int) and\n> +std::abs(float) while math.h defines abs(int) and fabs(float). For this reason\n> +the inclusion of is preferred over .\n\nI'd split this in two paragraphs, with the first one being the existing\ntext minus the penultimate sentence. Code shall not rely on the optional\ndeclarations in any case, there's no exception to that rule. It could\nbecome\n\n\n while the later are named . The C++ headers declare names in the\n std namespace, and may declare the same names in the global namespace. The C\n compatibility headers declare names in the global namespace, and may declare\n-the same names in the std namespace. Usage of the C compatibility headers is\n-strongly preferred. Code shall not rely on the optional declaration of names in\n-the global or std namespace.\n+the same names in the std namespace. Code shall not rely on the optional\n+declaration of names in the global or std namespace.\n+\n+Usage of the C compatibility headers is preferred, except for the math.h header.\n+Where math.h defines separate functions for different argument types (e.g.\n+abs(int), labs(long int), fabs(double) and fabsf(float)) and requires the\n+developer to pick the right function, cmath defines overloaded functions\n+(std::abs(int), std::abs(long int), std::abs(double) and std::abs(float) and let\n+the compiler select the right function. This avoids potential errors such as\n+calling abs(int) with a float argument, performing an unwanted implicit integer\n+conversion. For this reason, cmath is preferred over math.h.\n\n\nAnd we should also add\n\ndiff --git a/utils/checkstyle.py b/utils/checkstyle.py\nindex b594a19aed5b..d5dc26c0f666 100755\n--- a/utils/checkstyle.py\n+++ b/utils/checkstyle.py\n@@ -244,9 +244,9 @@ class IncludeChecker(StyleChecker):\n patterns = ('*.cpp', '*.h')\n\n headers = ('assert', 'ctype', 'errno', 'fenv', 'float', 'inttypes',\n- 'limits', 'locale', 'math', 'setjmp', 'signal', 'stdarg',\n- 'stddef', 'stdint', 'stdio', 'stdlib', 'string', 'time', 'uchar',\n- 'wchar', 'wctype')\n+ 'limits', 'locale', 'setjmp', 'signal', 'stdarg', 'stddef',\n+ 'stdint', 'stdio', 'stdlib', 'string', 'time', 'uchar', 'wchar',\n+ 'wctype')\n include_regex = re.compile('^#include ')\n\n def __init__(self, content):\n\n> > > Fixes: 968ab9bad0ed (\"libcamera: ipu3: imgu: Calculate ImgU pipe configuration\")\n> > > Signed-off-by: Jacopo Mondi \n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > Please push :-)\n> \n> Thanks\n>\n> > > ---\n> > > src/libcamera/pipeline/ipu3/imgu.cpp | 2 +-\n> > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > > index b7593ceb3672..eb829e096561 100644\n> > > --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > > @@ -7,8 +7,8 @@\n> > >\n> > > #include \"imgu.h\"\n> > >\n> > > +#include \n> > > #include \n> > > -#include \n> > >\n> > > #include \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 2E727BD86F\n\tfor ;\n\tWed, 5 Aug 2020 14:45:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B7D5F605A8;\n\tWed, 5 Aug 2020 16:45:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 90A466039D\n\tfor ;\n\tWed, 5 Aug 2020 16:45:37 +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 F285F2C0;\n\tWed, 5 Aug 2020 16:45:36 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RxET17ZT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596638737;\n\tbh=8F4vJsYOMFm0UR7OMcN7TWPeljVRzy5MpasO0LUC6bE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RxET17ZTTInqZh+p1sRjQwCt/g7phJSGJufaKolCvkAKLI3lK6pVPH4dkzAsakWLl\n\t31d6cEv41DHTh3sOw1JEX960N4TfilLSjuGyQiabCsAmr62+fvFmUqg4vTN5EmfFIO\n\tbjF6eALDf8YaPggwWssbuMgL1HxHcLn+L3vZfTjk=","Date":"Wed, 5 Aug 2020 17:45:25 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200805144525.GH6751@pendragon.ideasonboard.com>","References":"<20200803162811.77634-1-jacopo@jmondi.org>\n\t<20200804230034.GS6075@pendragon.ideasonboard.com>\n\t<20200805071029.aqmszycnsguasomm@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200805071029.aqmszycnsguasomm@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipu3: Fix compilation\n\tissues with gcc5","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]