[{"id":245,"web_url":"https://patchwork.libcamera.org/comment/245/","msgid":"<f44ba912-348a-2257-5dca-7b490c2264ac@ideasonboard.com>","date":"2019-01-08T10:34:50","subject":"Re: [libcamera-devel] [PATCH v2 02/11] libcamera: log: Add an\n\tASSERT macro","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 07/01/2019 23:11, Laurent Pinchart wrote:\n> The ASSERT() macro is similar to the assert() macro defined by the C\n> standard, but uses the libcamera logging infrastructure.\n> \n\nOnly discussion on this patch below - no blocker.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n> Changes since v1:\n> \n> - Fix syntax error in ASSERT() macro\n> - Fix NDEBUG conditional compilation\n> - Fix typo in documentation\n> - Fix ASSERT() condition check\n> ---\n>  src/libcamera/include/log.h |  9 +++++++++\n>  src/libcamera/log.cpp       | 16 ++++++++++++++++\n>  2 files changed, 25 insertions(+)\n> \n> diff --git a/src/libcamera/include/log.h b/src/libcamera/include/log.h\n> index 03842be02d0e..c1af3741ffee 100644\n> --- a/src/libcamera/include/log.h\n> +++ b/src/libcamera/include/log.h\n> @@ -36,6 +36,15 @@ private:\n>  \n>  #define LOG(severity) LogMessage(__FILE__, __LINE__, Log##severity).stream()\n>  \n> +#ifndef NDEBUG\n\nI'm interested in your choice of negating the DEBUG symbol here.\n   (i.e. instead of using \"#ifdef DEBUG\")\n\nWould you expect us to keep ASSERTs always on by default (i.e. for users\nof the library?) or would you foresee a release defining NDEBUG.\n\nI guess one benefit of negating this - means that by default anyone self\ncompiling the library (and thus developing) gets debug assertions\nenabled, while a release binary can specify the NDEBUG as part of any\nautomated build process if necessary.\n\n\n\nWould the LogDebug level also be compiled out if NDEBUG is defined?\n (not relevant for this patch, just curious as to your opinion)\n\n\n\n\n> +#define ASSERT(condition) static_cast<void>(({\t\t\t\t\\\n> +\tif (!(condition))\t\t\t\t\t\t\t\\\n> +\t\tLOG(Fatal) << \"assertion \\\"\" #condition \"\\\" failed\";\t\\\n> +}))\n> +#else\n> +#define ASSERT(condition) static_cast<void>(false && (condition))\n> +#endif\n> +\n>  } /* namespace libcamera */\n>  \n>  #endif /* __LIBCAMERA_LOG_H__ */\n> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> index a5823c64eaa6..6785d371449e 100644\n> --- a/src/libcamera/log.cpp\n> +++ b/src/libcamera/log.cpp\n> @@ -48,6 +48,22 @@ namespace libcamera {\n>   * terminates immediately after printing the message.\n>   */\n>  \n> +/**\n> + * \\def ASSERT(condition)\n> + * \\brief Abort program execution if assertion fails\n> + *\n> + * If \\a condition is false, ASSERT() logs an error message with the Fatal log\n> + * level and aborts program execution.\n> + *\n> + * If the macro NDEBUG is defined before including log.h, ASSERT() generates no\n> + * code.\n> + *\n> + * Using conditions that have side effects with ASSERT() is not recommended, as\n> + * these effects would depend on whether NDEBUG is defined or not. Similarly,\n> + * ASSERT() should not be used to check for errors that can occur under normal\n> + * conditions as those checks would then be removed when compiling with NDEBUG.\n> + */\n> +\n>  static const char *log_severity_name(LogSeverity severity)\n>  {\n>  \tstatic const char * const names[] = {\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4304B60B2D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Jan 2019 11:34:53 +0100 (CET)","from [192.168.0.21]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9A03E586;\n\tTue,  8 Jan 2019 11:34:52 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546943692;\n\tbh=84YDZ38xXvCWbJoj+hPgC42qdgogoN/EIOTxYUsCGTw=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=X6Me1sykecO6alojHZ2Y5b+eFgYwIUxMS2zd2FLfOeNAL78Dbyrf5G7AeJQWq1s1g\n\tpfE5ZT/UP4+tmpJjTJDdBOyV8SWM0OcPnbBDVH0lDIqyuJ9LeWTwpsc96a/l/JwDVg\n\tu6WF27znpz9jN5G4JdenRgp1j1BKHMVPf7JL2f+k=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190107231151.23291-1-laurent.pinchart@ideasonboard.com>\n\t<20190107231151.23291-3-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<f44ba912-348a-2257-5dca-7b490c2264ac@ideasonboard.com>","Date":"Tue, 8 Jan 2019 10:34:50 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<20190107231151.23291-3-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 02/11] libcamera: log: Add an\n\tASSERT macro","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 08 Jan 2019 10:34:53 -0000"}},{"id":248,"web_url":"https://patchwork.libcamera.org/comment/248/","msgid":"<1944222.QSAtyVubOv@avalon>","date":"2019-01-08T10:59:42","subject":"Re: [libcamera-devel] [PATCH v2 02/11] libcamera: log: Add an\n\tASSERT macro","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tuesday, 8 January 2019 12:34:50 EET Kieran Bingham wrote:\n> On 07/01/2019 23:11, Laurent Pinchart wrote:\n> > The ASSERT() macro is similar to the assert() macro defined by the C\n> > standard, but uses the libcamera logging infrastructure.\n> \n> Only discussion on this patch below - no blocker.\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> > Changes since v1:\n> > \n> > - Fix syntax error in ASSERT() macro\n> > - Fix NDEBUG conditional compilation\n> > - Fix typo in documentation\n> > - Fix ASSERT() condition check\n> > ---\n> > \n> >  src/libcamera/include/log.h |  9 +++++++++\n> >  src/libcamera/log.cpp       | 16 ++++++++++++++++\n> >  2 files changed, 25 insertions(+)\n> > \n> > diff --git a/src/libcamera/include/log.h b/src/libcamera/include/log.h\n> > index 03842be02d0e..c1af3741ffee 100644\n> > --- a/src/libcamera/include/log.h\n> > +++ b/src/libcamera/include/log.h\n> > \n> > @@ -36,6 +36,15 @@ private:\n> >  #define LOG(severity) LogMessage(__FILE__, __LINE__,\n> >  Log##severity).stream()> \n> > +#ifndef NDEBUG\n> \n> I'm interested in your choice of negating the DEBUG symbol here.\n>    (i.e. instead of using \"#ifdef DEBUG\")\n\nIt's how the POSIX, C and C++ standards handle assert():\n\nhttp://man7.org/linux/man-pages/man3/assert.3.html\nhttp://www.cplusplus.com/reference/cassert/assert/\n\n> Would you expect us to keep ASSERTs always on by default (i.e. for users\n> of the library?) or would you foresee a release defining NDEBUG.\n\nI foresee a release defining NDEBUG.\n\n> I guess one benefit of negating this - means that by default anyone self\n> compiling the library (and thus developing) gets debug assertions\n> enabled, while a release binary can specify the NDEBUG as part of any\n> automated build process if necessary.\n\nI don't know what the rationale behind the NDEBUG vs. DEBUG choice was, but \nthis would be a good one :-)\n\n> Would the LogDebug level also be compiled out if NDEBUG is defined?\n>  (not relevant for this patch, just curious as to your opinion)\n\nI'm tempted to keep it, as it's useful to debug applications using libcamera \nin addition to debugging libcamera itself.\n\n> > +#define ASSERT(condition) static_cast<void>(({\t\t\t\t\\\n> > +\tif (!(condition))\t\t\t\t\t\t\t\\\n> > +\t\tLOG(Fatal) << \"assertion \\\"\" #condition \"\\\" failed\";\t\\\n> > +}))\n> > +#else\n> > +#define ASSERT(condition) static_cast<void>(false && (condition))\n> > +#endif\n> > +\n> >  } /* namespace libcamera */\n> >  \n> >  #endif /* __LIBCAMERA_LOG_H__ */\n> > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> > index a5823c64eaa6..6785d371449e 100644\n> > --- a/src/libcamera/log.cpp\n> > +++ b/src/libcamera/log.cpp\n> > @@ -48,6 +48,22 @@ namespace libcamera {\n> > \n> >   * terminates immediately after printing the message.\n> >   */\n> > \n> > +/**\n> > + * \\def ASSERT(condition)\n> > + * \\brief Abort program execution if assertion fails\n> > + *\n> > + * If \\a condition is false, ASSERT() logs an error message with the\n> > Fatal log + * level and aborts program execution.\n> > + *\n> > + * If the macro NDEBUG is defined before including log.h, ASSERT()\n> > generates no + * code.\n> > + *\n> > + * Using conditions that have side effects with ASSERT() is not\n> > recommended, as + * these effects would depend on whether NDEBUG is\n> > defined or not. Similarly, + * ASSERT() should not be used to check for\n> > errors that can occur under normal + * conditions as those checks would\n> > then be removed when compiling with NDEBUG. + */\n> > +\n> > \n> >  static const char *log_severity_name(LogSeverity severity)\n> >  {\n> >  \n> >  \tstatic const char * const names[] = {","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 94FE260B2D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Jan 2019 11:58:34 +0100 (CET)","from avalon.localnet (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 16138586;\n\tTue,  8 Jan 2019 11:58:34 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546945114;\n\tbh=+3lYFDyFfXnkH0qTGXWRkIW25qhl21bby/U2pvECTrU=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=UdzBx679myDOJW2vrds6MLy84cEMwGUuI5n9ukgtjOKnX8wBqty9xrv3GPNjjfRgV\n\ttgnhzrI6qC/QhXwooacZ6eL5Ib217f4dSQOa9jjk5OMgTDP2PTCcIZMWuwHW6QHFsQ\n\trzyzO/yvWT6urLupIH3sVh+Hvf8o24NOax5YdcpU=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Date":"Tue, 08 Jan 2019 12:59:42 +0200","Message-ID":"<1944222.QSAtyVubOv@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<f44ba912-348a-2257-5dca-7b490c2264ac@ideasonboard.com>","References":"<20190107231151.23291-1-laurent.pinchart@ideasonboard.com>\n\t<20190107231151.23291-3-laurent.pinchart@ideasonboard.com>\n\t<f44ba912-348a-2257-5dca-7b490c2264ac@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=\"iso-8859-1\"","Subject":"Re: [libcamera-devel] [PATCH v2 02/11] libcamera: log: Add an\n\tASSERT macro","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 08 Jan 2019 10:58:34 -0000"}},{"id":249,"web_url":"https://patchwork.libcamera.org/comment/249/","msgid":"<253715ad-78bd-5d5e-7249-4bc2b49af0df@ideasonboard.com>","date":"2019-01-08T11:03:24","subject":"Re: [libcamera-devel] [PATCH v2 02/11] libcamera: log: Add an\n\tASSERT macro","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 08/01/2019 10:59, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Tuesday, 8 January 2019 12:34:50 EET Kieran Bingham wrote:\n>> On 07/01/2019 23:11, Laurent Pinchart wrote:\n>>> The ASSERT() macro is similar to the assert() macro defined by the C\n>>> standard, but uses the libcamera logging infrastructure.\n>>\n>> Only discussion on this patch below - no blocker.\n>>\n>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>\n>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n>>> ---\n>>> Changes since v1:\n>>>\n>>> - Fix syntax error in ASSERT() macro\n>>> - Fix NDEBUG conditional compilation\n>>> - Fix typo in documentation\n>>> - Fix ASSERT() condition check\n>>> ---\n>>>\n>>>  src/libcamera/include/log.h |  9 +++++++++\n>>>  src/libcamera/log.cpp       | 16 ++++++++++++++++\n>>>  2 files changed, 25 insertions(+)\n>>>\n>>> diff --git a/src/libcamera/include/log.h b/src/libcamera/include/log.h\n>>> index 03842be02d0e..c1af3741ffee 100644\n>>> --- a/src/libcamera/include/log.h\n>>> +++ b/src/libcamera/include/log.h\n>>>\n>>> @@ -36,6 +36,15 @@ private:\n>>>  #define LOG(severity) LogMessage(__FILE__, __LINE__,\n>>>  Log##severity).stream()> \n>>> +#ifndef NDEBUG\n>>\n>> I'm interested in your choice of negating the DEBUG symbol here.\n>>    (i.e. instead of using \"#ifdef DEBUG\")\n> \n> It's how the POSIX, C and C++ standards handle assert():\n> \n> http://man7.org/linux/man-pages/man3/assert.3.html\n> http://www.cplusplus.com/reference/cassert/assert/\n\nAha yes - of course ... that's a good reason :)\n\n\n>> Would you expect us to keep ASSERTs always on by default (i.e. for users\n>> of the library?) or would you foresee a release defining NDEBUG.\n> \n> I foresee a release defining NDEBUG.\n> \n>> I guess one benefit of negating this - means that by default anyone self\n>> compiling the library (and thus developing) gets debug assertions\n>> enabled, while a release binary can specify the NDEBUG as part of any\n>> automated build process if necessary.\n> \n> I don't know what the rationale behind the NDEBUG vs. DEBUG choice was, but \n> this would be a good one :-)\n> \n>> Would the LogDebug level also be compiled out if NDEBUG is defined?\n>>  (not relevant for this patch, just curious as to your opinion)\n> \n> I'm tempted to keep it, as it's useful to debug applications using libcamera \n> in addition to debugging libcamera itself.\n\nYes, we'll have different log levels provided at runtime anyway so it's\nall filterable.\n\nClears up my thoughts anyway, thanks.\n\n--\nKieran\n\n\n\n\n\n>>> +#define ASSERT(condition) static_cast<void>(({\t\t\t\t\\\n>>> +\tif (!(condition))\t\t\t\t\t\t\t\\\n>>> +\t\tLOG(Fatal) << \"assertion \\\"\" #condition \"\\\" failed\";\t\\\n>>> +}))\n>>> +#else\n>>> +#define ASSERT(condition) static_cast<void>(false && (condition))\n>>> +#endif\n>>> +\n>>>  } /* namespace libcamera */\n>>>  \n>>>  #endif /* __LIBCAMERA_LOG_H__ */\n>>> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n>>> index a5823c64eaa6..6785d371449e 100644\n>>> --- a/src/libcamera/log.cpp\n>>> +++ b/src/libcamera/log.cpp\n>>> @@ -48,6 +48,22 @@ namespace libcamera {\n>>>\n>>>   * terminates immediately after printing the message.\n>>>   */\n>>>\n>>> +/**\n>>> + * \\def ASSERT(condition)\n>>> + * \\brief Abort program execution if assertion fails\n>>> + *\n>>> + * If \\a condition is false, ASSERT() logs an error message with the\n>>> Fatal log + * level and aborts program execution.\n>>> + *\n>>> + * If the macro NDEBUG is defined before including log.h, ASSERT()\n>>> generates no + * code.\n>>> + *\n>>> + * Using conditions that have side effects with ASSERT() is not\n>>> recommended, as + * these effects would depend on whether NDEBUG is\n>>> defined or not. Similarly, + * ASSERT() should not be used to check for\n>>> errors that can occur under normal + * conditions as those checks would\n>>> then be removed when compiling with NDEBUG. + */\n>>> +\n>>>\n>>>  static const char *log_severity_name(LogSeverity severity)\n>>>  {\n>>>  \n>>>  \tstatic const char * const names[] = {\n> \n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 673BC60B0B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Jan 2019 12:03:27 +0100 (CET)","from [192.168.0.21]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C9D61586;\n\tTue,  8 Jan 2019 12:03:26 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546945407;\n\tbh=aLLcw3XYiUwJl82Gi48ju5HwLukjGgpWw5Fbv3BfkWo=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=uwu9FQ/AwY8UpC3euhMMp8EvwzSCLLfOq0R6XZ0FKfRvo7slPkHM7IbLAVdIPGrGj\n\tMcVNBZOOUgwrLNBOcedvCKBCuUh4ZLNWiJZV/hIdxMl7/fQ6KEDI88HAn0S6Ufcybz\n\thzf+tnXm6cMZ1Nkp4Er4ctpxzU+2Vdt2zQR5m01c=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20190107231151.23291-1-laurent.pinchart@ideasonboard.com>\n\t<20190107231151.23291-3-laurent.pinchart@ideasonboard.com>\n\t<f44ba912-348a-2257-5dca-7b490c2264ac@ideasonboard.com>\n\t<1944222.QSAtyVubOv@avalon>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<253715ad-78bd-5d5e-7249-4bc2b49af0df@ideasonboard.com>","Date":"Tue, 8 Jan 2019 11:03:24 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<1944222.QSAtyVubOv@avalon>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 02/11] libcamera: log: Add an\n\tASSERT macro","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 08 Jan 2019 11:03:27 -0000"}}]