[{"id":440,"web_url":"https://patchwork.libcamera.org/comment/440/","msgid":"<20190121103634.GE6484@bigcity.dyn.berto.se>","date":"2019-01-21T10:36:34","subject":"Re: [libcamera-devel] [PATCH] test: v4l2_device: Provide compile\n\ttime assertions","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran,\n\nNice idea for a test :-)\n\nOn 2019-01-21 10:28:26 +0000, Kieran Bingham wrote:\n> The V4L2Capability structure is inherited from struct v4l2_capability only\n> to provide an interface. It must not extend the type or data.\n> \n> Enforce this with a static assertion with sizeof() comparisons.\n> \n> There is no need here for a specific test binary which will always return\n> TEST_PASS when compiled, as this test failure will be caught at compile time.\n> In light of this - the static compile time assertion is added to the\n> V4L2DeviceTest base class.\n> \n> Should there be a large number of static assertions required, they could be\n> moved to their own unit for clarity.\n> \n> Signed-off-by: Kieran Bingham \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> test/v4l2_device/v4l2_device_test.cpp | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp\n> index 362553712caa..97876f8d65db 100644\n> --- a/test/v4l2_device/v4l2_device_test.cpp\n> +++ b/test/v4l2_device/v4l2_device_test.cpp\n> @@ -5,6 +5,7 @@\n> * libcamera V4L2 API tests\n> */\n> \n> +#include \n> #include \n> #include \n> \n> @@ -41,3 +42,8 @@ void V4L2DeviceTest::cleanup()\n> {\n> \tdelete dev_;\n> };\n> +\n> +/* Static compile time assertion tests */\n> +\n> +static_assert(sizeof(struct v4l2_capability) == sizeof(struct V4L2Capability),\n> +\t \"V4L2Capability must match v4l2_capability size\");\n> -- \n> 2.17.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3FBDF60B21\n\tfor ;\n\tMon, 21 Jan 2019 11:36:36 +0100 (CET)","by mail-lf1-x142.google.com with SMTP id a8so15119671lfk.5\n\tfor ;\n\tMon, 21 Jan 2019 02:36:36 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tu12-v6sm2214316ljk.79.2019.01.21.02.36.34\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 21 Jan 2019 02:36:34 -0800 (PST)"],"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=HI3+ptxKYN5pOi93qw17E7tH+K/T9fxt3A+JrCRqwU0=;\n\tb=SYmPrsks+oTEtmKkqqwj8gpjZ/r/4E/30Fs98q5zKWdqiKiACo1ZagU1R/zP2xFf3P\n\tS93Z0fDAX25PqXwMq57WIQkngonGTmpLO1YBl5eLODC+dBzkqFuXp9lY/A1x8woc664w\n\tjcUUWSJ9U7a62eizID5moLY1ETR06LZBODI/GUGAk9RdE1vyiezjV1vFUC7k7Ew/wVV6\n\tDqfWTYiTxAj5JawhgbCvLBcOi2W0E1DbqvvRIUr7dqmPBz8O37hL4qtEKQ55GaQ4T6fi\n\txtum9eiW/JMrVN6fXL0wjXm9vCinrAadRMOXdmJnuxtmgEASh1oIKGWjdbNlkn8m3Kuf\n\t3R4A==","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=HI3+ptxKYN5pOi93qw17E7tH+K/T9fxt3A+JrCRqwU0=;\n\tb=K+QwKLzlsc2Q90F/+UqG9A4VqL1JhZc8HjcTMHKpPr9UnY2eAwf4yBEerESl1Hd1jB\n\t53PGrXnT7jvFMojT3K/IOnhxWYZMICzKpTuGyEt34Pj1nVP7T8SDTGLLw3J1M0CcOc2W\n\toxZSQ3AxKP3SDuiaLbyDELvfTCQ/OfcBwOv5gSl4sj+An9S0hG+Pqw9EkZQIkln397V3\n\tqMc7QGH8Zt/5+MrrzUt/TG8jn4Y65KVvVvGM7/pMv+ldQHvrp6ci1pMA3BSd+R0CalVh\n\tPAmlt7yXFlzwF54WdbhbXR+4p3jy46hJhVhUV6UERBkYLDivbsCm2p+GhjYsMrW2YOYp\n\tAQZA==","X-Gm-Message-State":"AJcUukfJ42udHkVQcMtdWGdGXXYgz9dy7sTzE6kFCw6QC8ueuCKCXiDX\n\tzUsZm/AD4J73erDJiqkaS2bypqQNnU4=","X-Google-Smtp-Source":"ALg8bN6wm1BYmyC7EK91KdiZxGZkZmedVLp64LG4j++ASnZHPJFGCtPByrLklztW1BnSMSgGQlB4xg==","X-Received":"by 2002:a19:3809:: with SMTP id\n\tf9mr17011402lfa.148.1548066995394; \n\tMon, 21 Jan 2019 02:36:35 -0800 (PST)","Date":"Mon, 21 Jan 2019 11:36:34 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Kieran Bingham ","Cc":"LibCamera Devel ","Message-ID":"<20190121103634.GE6484@bigcity.dyn.berto.se>","References":"<20190121102826.16873-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190121102826.16873-1-kieran.bingham@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] test: v4l2_device: Provide compile\n\ttime assertions","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, 21 Jan 2019 10:36:36 -0000"}},{"id":441,"web_url":"https://patchwork.libcamera.org/comment/441/","msgid":"<94903e8c-b3b9-ebe5-59f0-51e0e181a28e@ideasonboard.com>","date":"2019-01-21T10:39:16","subject":"Re: [libcamera-devel] [PATCH] test: v4l2_device: Provide compile\n\ttime assertions","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 21/01/2019 10:36, Niklas Söderlund wrote:\n> Hi Kieran,\n> \n> Nice idea for a test :-)\n>\n> On 2019-01-21 10:28:26 +0000, Kieran Bingham wrote:\n>> The V4L2Capability structure is inherited from struct v4l2_capability only\n>> to provide an interface. It must not extend the type or data.\n>>\n>> Enforce this with a static assertion with sizeof() comparisons.\n>>\n>> There is no need here for a specific test binary which will always return\n>> TEST_PASS when compiled, as this test failure will be caught at compile time.\n>> In light of this - the static compile time assertion is added to the\n>> V4L2DeviceTest base class.\n>>\n>> Should there be a large number of static assertions required, they could be\n>> moved to their own unit for clarity.\n>>\n>> Signed-off-by: Kieran Bingham \n> \n> Reviewed-by: Niklas Söderlund \n\nThanks,\n\nI hope this alleviates some of your earlier concerns about the method of\nimplementing the interface to interact kernel structures ...\n\nIt's to define what functions can operate on a structure - certainly not\nto to extend that structure in any way.\n\n--\nKieran\n\n> \n>> ---\n>> test/v4l2_device/v4l2_device_test.cpp | 6 ++++++\n>> 1 file changed, 6 insertions(+)\n>>\n>> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp\n>> index 362553712caa..97876f8d65db 100644\n>> --- a/test/v4l2_device/v4l2_device_test.cpp\n>> +++ b/test/v4l2_device/v4l2_device_test.cpp\n>> @@ -5,6 +5,7 @@\n>> * libcamera V4L2 API tests\n>> */\n>> \n>> +#include \n>> #include \n>> #include \n>> \n>> @@ -41,3 +42,8 @@ void V4L2DeviceTest::cleanup()\n>> {\n>> \tdelete dev_;\n>> };\n>> +\n>> +/* Static compile time assertion tests */\n>> +\n>> +static_assert(sizeof(struct v4l2_capability) == sizeof(struct V4L2Capability),\n>> +\t \"V4L2Capability must match v4l2_capability size\");\n>> -- \n>> 2.17.1\n>>\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel\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 E8A8160B21\n\tfor ;\n\tMon, 21 Jan 2019 11:39:19 +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 6047C53E;\n\tMon, 21 Jan 2019 11:39:19 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548067159;\n\tbh=zykqzW/GIccu5iFfnMqwCIciuubeOq3GiIB3No74b8A=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=cWDHoKjS6m1BeiurkemyFxPNU0NFTQ3H1i1Zdww92EiIImcMHAzElDsqMORC+O7ST\n\tGUOcr5pSHEE8hWKLY5oyRifnSLyi8Go/m3QvZSA2pBYXjnbveMg1a/2hxX5lfxq/Qr\n\t6IOmV2iKds99/F8vgm55Xwo1g5OivsRmNxjeBEiU=","Reply-To":"kieran.bingham@ideasonboard.com","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","Cc":"LibCamera Devel ","References":"<20190121102826.16873-1-kieran.bingham@ideasonboard.com>\n\t<20190121103634.GE6484@bigcity.dyn.berto.se>","From":"Kieran Bingham ","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":"<94903e8c-b3b9-ebe5-59f0-51e0e181a28e@ideasonboard.com>","Date":"Mon, 21 Jan 2019 10:39:16 +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":"<20190121103634.GE6484@bigcity.dyn.berto.se>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] test: v4l2_device: Provide compile\n\ttime assertions","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, 21 Jan 2019 10:39:20 -0000"}},{"id":446,"web_url":"https://patchwork.libcamera.org/comment/446/","msgid":"<20190121121525.GC12780@pendragon.ideasonboard.com>","date":"2019-01-21T12:15:25","subject":"Re: [libcamera-devel] [PATCH] test: v4l2_device: Provide compile\n\ttime assertions","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 Mon, Jan 21, 2019 at 10:28:26AM +0000, Kieran Bingham wrote:\n> The V4L2Capability structure is inherited from struct v4l2_capability only\n> to provide an interface. It must not extend the type or data.\n> \n> Enforce this with a static assertion with sizeof() comparisons.\n> \n> There is no need here for a specific test binary which will always return\n> TEST_PASS when compiled, as this test failure will be caught at compile time.\n> In light of this - the static compile time assertion is added to the\n> V4L2DeviceTest base class.\n> \n> Should there be a large number of static assertions required, they could be\n> moved to their own unit for clarity.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> test/v4l2_device/v4l2_device_test.cpp | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp\n> index 362553712caa..97876f8d65db 100644\n> --- a/test/v4l2_device/v4l2_device_test.cpp\n> +++ b/test/v4l2_device/v4l2_device_test.cpp\n> @@ -5,6 +5,7 @@\n> * libcamera V4L2 API tests\n> */\n> \n> +#include \n> #include \n> #include \n> \n> @@ -41,3 +42,8 @@ void V4L2DeviceTest::cleanup()\n> {\n> \tdelete dev_;\n> };\n> +\n> +/* Static compile time assertion tests */\n> +\n> +static_assert(sizeof(struct v4l2_capability) == sizeof(struct V4L2Capability),\n> +\t \"V4L2Capability must match v4l2_capability size\");\n\nI'm still not sure if we really need this, as this is more a check of\nour C++ understanding than of our implementation :-) If you insist, I\nprefer this check to be in a test case than in libcamera where it\noriginally was, so I won't push back too hard. The error message may\nneed some rewording though, as it sounds like it's something we can fix.\n\nBy the way, do we really require the two to be equal ? V4L2Capability\ncould include more members (I don't see a real use case for that now,\nbut that's imaginable), in which case it would be laid ou with\nv4l2_capability first followed by other members, and still work fine.","headers":{"Return-Path":"","Received":["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 F237F60C7F\n\tfor ;\n\tMon, 21 Jan 2019 13:17:35 +0100 (CET)","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 57EBF53E;\n\tMon, 21 Jan 2019 13:17:35 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548073055;\n\tbh=v9mVzI4qCyHZ2Ybhw6MUiDqbTZ2s+Q/N0xUbHhbixUo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KKqXzl67dptgn2HUuX05MPKph2+2HzsNBNmghCR4LL0n1yWZavMwOsREzE2yH0mve\n\tOoutqmKlB7dxr1jxv0OFNjw8Y6evImxZioqB7DqOueb+Oc23F+WoPDNkqEYLe1xrtn\n\tjERuQn1lJPlPUPu6ihtLIjZJtSj50oiFS+2phuSM=","Date":"Mon, 21 Jan 2019 14:15:25 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"LibCamera Devel ","Message-ID":"<20190121121525.GC12780@pendragon.ideasonboard.com>","References":"<20190121102826.16873-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190121102826.16873-1-kieran.bingham@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] test: v4l2_device: Provide compile\n\ttime assertions","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, 21 Jan 2019 12:17:36 -0000"}},{"id":448,"web_url":"https://patchwork.libcamera.org/comment/448/","msgid":"<7dc2fada-c2ea-e8ea-4623-eda1f0efd3bd@ideasonboard.com>","date":"2019-01-21T12:39:53","subject":"Re: [libcamera-devel] [PATCH] test: v4l2_device: Provide compile\n\ttime assertions","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 21/01/2019 12:15, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Mon, Jan 21, 2019 at 10:28:26AM +0000, Kieran Bingham wrote:\n>> The V4L2Capability structure is inherited from struct v4l2_capability only\n>> to provide an interface. It must not extend the type or data.\n>>\n>> Enforce this with a static assertion with sizeof() comparisons.\n>>\n>> There is no need here for a specific test binary which will always return\n>> TEST_PASS when compiled, as this test failure will be caught at compile time.\n>> In light of this - the static compile time assertion is added to the\n>> V4L2DeviceTest base class.\n>>\n>> Should there be a large number of static assertions required, they could be\n>> moved to their own unit for clarity.\n>>\n>> Signed-off-by: Kieran Bingham \n>> ---\n>> test/v4l2_device/v4l2_device_test.cpp | 6 ++++++\n>> 1 file changed, 6 insertions(+)\n>>\n>> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp\n>> index 362553712caa..97876f8d65db 100644\n>> --- a/test/v4l2_device/v4l2_device_test.cpp\n>> +++ b/test/v4l2_device/v4l2_device_test.cpp\n>> @@ -5,6 +5,7 @@\n>> * libcamera V4L2 API tests\n>> */\n>> \n>> +#include \n>> #include \n>> #include \n>> \n>> @@ -41,3 +42,8 @@ void V4L2DeviceTest::cleanup()\n>> {\n>> \tdelete dev_;\n>> };\n>> +\n>> +/* Static compile time assertion tests */\n>> +\n>> +static_assert(sizeof(struct v4l2_capability) == sizeof(struct V4L2Capability),\n>> +\t \"V4L2Capability must match v4l2_capability size\");\n> \n> I'm still not sure if we really need this, as this is more a check of\n> our C++ understanding than of our implementation :-) If you insist, I\n> prefer this check to be in a test case than in libcamera where it\n> originally was, so I won't push back too hard. The error message may\n> need some rewording though, as it sounds like it's something we can fix.\n\nIt's not a check of our understanding - it's a contract written in the\ncode - that the type V4L2Capability is used directly as a\nv4l2_capability and as such must remain compatible.\n\n\nIf this assertion fires, then it should be something to be fixed (or\nthat developer takes responsibility for removing this check and all\nguarantees I make are removed)\n\nI'm not expecting that the compiler might break the sizes arbitrarily -\nI'm ensuring that if a developer comes along and changes the size of the\nderived structure (s)he will get a warning, and be told off politely for\nchanging the size of the V4L2Capability.\n\n\n> By the way, do we really require the two to be equal ? V4L2Capability\n> could include more members (I don't see a real use case for that now,\n> but that's imaginable), in which case it would be laid ou with\n> v4l2_capability first followed by other members, and still work fine.\n\nYes, I agree - and originally I had planned for that to be possible.\nfor instance - to cache the appropriate capabilities variable (i.e. if\nthe device capabilities are to be referenced instead)\n\nI plan to add a function which I believe will be inlined to handle that\ninstead, but it states the point.\n\nHowever, if this happens - it could get messy - if the base class (the\nkernel structure) members are overridden by the derived class people\nmight not get the behaviour they expect. (ioctl writes to the base class\nstruct, user reads from derived override)\n\nAnyway - the point of this assert it to state the intentions of the\ndefinition, and provide a warning if anyone breaks that assumption - not\nto assert that two objects which we know are currently the same size -\n\"are the same size\".","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 C396E60C7F\n\tfor ;\n\tMon, 21 Jan 2019 13:39:57 +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 1D44D53E;\n\tMon, 21 Jan 2019 13:39:57 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548074397;\n\tbh=lctAUdHlACVQtjzU71sGxXdK6/Or0h6oDTP+oZcx0hk=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=F3AppB9kfHTL5rpUx3pSvP/9aPkDc1prSeMCj6feMjot7H8zi3+chAbhcSJB8ACVS\n\t0WK9GFceFjfn/oDgHKDepcz4u/6fMhimmB8MYlxl3b+ZGnAeNW0nV7zxpdT1okZJ7E\n\tv+BuxJJir5peRHy8u0PYYZ5vNRYJr8IGkW83g7Ao=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ","Cc":"LibCamera Devel ","References":"<20190121102826.16873-1-kieran.bingham@ideasonboard.com>\n\t<20190121121525.GC12780@pendragon.ideasonboard.com>","From":"Kieran Bingham ","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":"<7dc2fada-c2ea-e8ea-4623-eda1f0efd3bd@ideasonboard.com>","Date":"Mon, 21 Jan 2019 12:39:53 +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":"<20190121121525.GC12780@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] test: v4l2_device: Provide compile\n\ttime assertions","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, 21 Jan 2019 12:39:57 -0000"}},{"id":450,"web_url":"https://patchwork.libcamera.org/comment/450/","msgid":"<20190121130626.GF12780@pendragon.ideasonboard.com>","date":"2019-01-21T13:06:26","subject":"Re: [libcamera-devel] [PATCH] test: v4l2_device: Provide compile\n\ttime assertions","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Jan 21, 2019 at 12:39:53PM +0000, Kieran Bingham wrote:\n> On 21/01/2019 12:15, Laurent Pinchart wrote:\n> > On Mon, Jan 21, 2019 at 10:28:26AM +0000, Kieran Bingham wrote:\n> >> The V4L2Capability structure is inherited from struct v4l2_capability only\n> >> to provide an interface. It must not extend the type or data.\n> >>\n> >> Enforce this with a static assertion with sizeof() comparisons.\n> >>\n> >> There is no need here for a specific test binary which will always return\n> >> TEST_PASS when compiled, as this test failure will be caught at compile time.\n> >> In light of this - the static compile time assertion is added to the\n> >> V4L2DeviceTest base class.\n> >>\n> >> Should there be a large number of static assertions required, they could be\n> >> moved to their own unit for clarity.\n> >>\n> >> Signed-off-by: Kieran Bingham \n> >> ---\n> >> test/v4l2_device/v4l2_device_test.cpp | 6 ++++++\n> >> 1 file changed, 6 insertions(+)\n> >>\n> >> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp\n> >> index 362553712caa..97876f8d65db 100644\n> >> --- a/test/v4l2_device/v4l2_device_test.cpp\n> >> +++ b/test/v4l2_device/v4l2_device_test.cpp\n> >> @@ -5,6 +5,7 @@\n> >> * libcamera V4L2 API tests\n> >> */\n> >> \n> >> +#include \n> >> #include \n> >> #include \n> >> \n> >> @@ -41,3 +42,8 @@ void V4L2DeviceTest::cleanup()\n> >> {\n> >> \tdelete dev_;\n> >> };\n> >> +\n> >> +/* Static compile time assertion tests */\n> >> +\n> >> +static_assert(sizeof(struct v4l2_capability) == sizeof(struct V4L2Capability),\n> >> +\t \"V4L2Capability must match v4l2_capability size\");\n> > \n> > I'm still not sure if we really need this, as this is more a check of\n> > our C++ understanding than of our implementation :-) If you insist, I\n> > prefer this check to be in a test case than in libcamera where it\n> > originally was, so I won't push back too hard. The error message may\n> > need some rewording though, as it sounds like it's something we can fix.\n> \n> It's not a check of our understanding - it's a contract written in the\n> code - that the type V4L2Capability is used directly as a\n> v4l2_capability and as such must remain compatible.\n\nBut that's guaranteed by C++ (isn't it ?), and we don't have tests for\nall contracts from the C++ standard :-)\n\n> If this assertion fires, then it should be something to be fixed (or\n> that developer takes responsibility for removing this check and all\n> guarantees I make are removed)\n> \n> I'm not expecting that the compiler might break the sizes arbitrarily -\n> I'm ensuring that if a developer comes along and changes the size of the\n> derived structure (s)he will get a warning, and be told off politely for\n> changing the size of the V4L2Capability.\n> \n> > By the way, do we really require the two to be equal ? V4L2Capability\n> > could include more members (I don't see a real use case for that now,\n> > but that's imaginable), in which case it would be laid ou with\n> > v4l2_capability first followed by other members, and still work fine.\n> \n> Yes, I agree - and originally I had planned for that to be possible.\n> for instance - to cache the appropriate capabilities variable (i.e. if\n> the device capabilities are to be referenced instead)\n> \n> I plan to add a function which I believe will be inlined to handle that\n> instead, but it states the point.\n> \n> However, if this happens - it could get messy - if the base class (the\n> kernel structure) members are overridden by the derived class people\n> might not get the behaviour they expect. (ioctl writes to the base class\n> struct, user reads from derived override)\n\nI don't see how that could happen, as C++ should guarantee that the\nobject will be layed out in memory with the base class first (again,\ndoesn't it ?).\n\n> Anyway - the point of this assert it to state the intentions of the\n> definition, and provide a warning if anyone breaks that assumption - not\n> to assert that two objects which we know are currently the same size -\n> \"are the same size\".\n\nMy question was if we should test if the compiler breaks assumptions\nthat come from the C++ standard :-)","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 2AA6E60C7F\n\tfor ;\n\tMon, 21 Jan 2019 14:06:28 +0100 (CET)","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 8D5E953E;\n\tMon, 21 Jan 2019 14:06:27 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548075987;\n\tbh=ElaOJ10bBgnxVFvHHjcEuG1+lJLeeMCq/P3ICS8RnVI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HDjGvjyO5GKJmXehetG5j8q4Uy99yR9c0cYhWafL67e4R9T+vGBUIoDshliVDSwr3\n\titHmiHEzFciLrCSTACJWzMG7Drs7brh1/QDUSrOSeOy8v5HKPtKu2SzONw1qSv0iI2\n\tv2eRHC0ABnxO5pIgSI3ox6yxevhsEW+3mZ3vEy6c=","Date":"Mon, 21 Jan 2019 15:06:26 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"LibCamera Devel ","Message-ID":"<20190121130626.GF12780@pendragon.ideasonboard.com>","References":"<20190121102826.16873-1-kieran.bingham@ideasonboard.com>\n\t<20190121121525.GC12780@pendragon.ideasonboard.com>\n\t<7dc2fada-c2ea-e8ea-4623-eda1f0efd3bd@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<7dc2fada-c2ea-e8ea-4623-eda1f0efd3bd@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] test: v4l2_device: Provide compile\n\ttime assertions","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, 21 Jan 2019 13:06:28 -0000"}}]