[{"id":13423,"web_url":"https://patchwork.libcamera.org/comment/13423/","msgid":"<20201023042306.GB4113@pendragon.ideasonboard.com>","date":"2020-10-23T04:23:06","subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Provide class.h","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 Thu, Oct 22, 2020 at 02:56:01PM +0100, Kieran Bingham wrote:\n> Provide a public header to define helpers for class definitions.\n> This initially implements macros to clearly define the deletion of\n> copy/move/assignment constructors/operators for classes to restrict\n> their capabilities explicitly.\n\nThanks for picking this up after my unsuccessful experimented with a\nnon-copyable base class. There's a few bikesheeding comments beow, but\nidea looks good.\n\nHave you considered including this in a more generic header, such as\nutils.h (or global.h, or .h) ? I don't foresee lots of new\nfeatures to be added to class.h, while I think we'll have more\nmiscellaneous macros and functions moving forward. It would be nice to\navoid creating micro-headers.\n\n> Signed-off-by: Kieran Bingham \n> ---\n> include/libcamera/class.h | 35 +++++++++++++++++++++++++++++++++++\n> include/libcamera/meson.build | 1 +\n> 2 files changed, 36 insertions(+)\n> create mode 100644 include/libcamera/class.h\n> \n> diff --git a/include/libcamera/class.h b/include/libcamera/class.h\n> new file mode 100644\n> index 000000000000..adcfa8860957\n> --- /dev/null\n> +++ b/include/libcamera/class.h\n> @@ -0,0 +1,35 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * class.h - class helpers\n> + */\n> +#ifndef __LIBCAMERA_CLASS_H__\n> +#define __LIBCAMERA_CLASS_H__\n> +\n> +/**\n> + * \\brief Delete the copy constructor and assignment operator.\n\nMissing \\param ?\n\n> + */\n> +#define DELETE_COPY_AND_ASSIGN(klass) \\\n\nMacros are unfortunately not part of namespaces, so to avoid a risk of\ncollision, I'd use a LIBCAMERA_ prefix. And to avoid the name getting\ntoo long, I think you can drop the _AND_ASSIGN suffix, as the macro is\ndisabling copying as a whole. Otherwise it should be\nDELETE_COPY_CONSTRUCT_AND_ASSIGN :-)\n\nAnother bikeshedding topic, I would have gone for DISABLE instead of\nDELETE to emphasize the purpose of the macro instead of how it's\nimplemented (not that libcamera has to care about this, but in older C++\nversions we would have needed to declare the functions as private as\nthere was no = delete).\n\n> +\t/* copy constructor. */ \\\n\nI think you can drop the comments, it's quite explicit already.\n\n> +\tklass(const klass &) = delete; \\\n> +\t/* copy assignment operator. */ \\\n> +\tklass &operator=(const klass &) = delete\n> +\n> +/**\n> + * \\brief Delete the move construtor and assignment operator.\n> + */\n> +#define DELETE_MOVE_AND_ASSIGN(klass) \\\n> +\t/* move constructor. */ \\\n> +\tklass(klass &&) = delete; \\\n> +\t/* move assignment operator. */ \\\n> +\tklass &operator=(klass &&) = delete\n> +\n> +/**\n> + * \\brief Delete all copy and move constructors, and assignment operators.\n> + */\n> +#define DELETE_COPY_MOVE_AND_ASSIGN(klass) \\\n> +\tDELETE_COPY_AND_ASSIGN(klass); \\\n> +\tDELETE_MOVE_AND_ASSIGN(klass)\n> +\n> +#endif /* __LIBCAMERA_CLASS_H__ */\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 3d5fc70134ad..b3363a6f735b 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -5,6 +5,7 @@ libcamera_public_headers = files([\n> 'buffer.h',\n> 'camera.h',\n> 'camera_manager.h',\n> + 'class.h',\n> 'controls.h',\n> 'event_dispatcher.h',\n> 'event_notifier.h',","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 07E29BDB13\n\tfor ;\n\tFri, 23 Oct 2020 04:23:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 62503615D4;\n\tFri, 23 Oct 2020 06:23:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 74BB56034E\n\tfor ;\n\tFri, 23 Oct 2020 06:23:52 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E2ACA53;\n\tFri, 23 Oct 2020 06:23:51 +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=\"duvgwbRD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603427032;\n\tbh=PBdTa3jwR/wLzJvAnuZqarqZIRQ/PnqKt1Sc/zN7jwM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=duvgwbRD54PH4JmG/AZpA9yHS2qkY23LeuCANjgpsqefhSObVXIt4qCPbQ6FwEyH1\n\tI5k6xMD5c4ivLtNzmMYvQnBZ2Mvr0qhyRtbkk1IUeGJ8UZwVlx0HeD+LmTf/PX/M5L\n\tItjRIXAvj2R2DtisQJiDiHXOWPMcuoGVv2y8RvbE=","Date":"Fri, 23 Oct 2020 07:23:06 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20201023042306.GB4113@pendragon.ideasonboard.com>","References":"<20201022135605.614240-1-kieran.bingham@ideasonboard.com>\n\t<20201022135605.614240-2-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201022135605.614240-2-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Provide class.h","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 ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13435,"web_url":"https://patchwork.libcamera.org/comment/13435/","msgid":"<577f983a-4a3b-eee4-b29c-c5c1343abeb2@ideasonboard.com>","date":"2020-10-23T08:16:55","subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Provide class.h","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 23/10/2020 05:23, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Thu, Oct 22, 2020 at 02:56:01PM +0100, Kieran Bingham wrote:\n>> Provide a public header to define helpers for class definitions.\n>> This initially implements macros to clearly define the deletion of\n>> copy/move/assignment constructors/operators for classes to restrict\n>> their capabilities explicitly.\n> \n> Thanks for picking this up after my unsuccessful experimented with a\n> non-copyable base class. There's a few bikesheeding comments beow, but\n> idea looks good.\n> \n> Have you considered including this in a more generic header, such as\n> utils.h (or global.h, or .h) ? I don't foresee lots of new\n> features to be added to class.h, while I think we'll have more\n> miscellaneous macros and functions moving forward. It would be nice to\n> avoid creating micro-headers.\n\nThat's why I chose class.h ...\n\nI thought this would cover more than just these macros.\n\nThrowing your argument back at you, You've recently posted a patch which\nadds 'extensible.h' ... and both these, and your extensible concepts are\nattributed to 'class' helpers, (yet these could not go in to\nextensible.h) ...\n\nSo ... Would you prefer to put your extensible types in class.h, or\nglobal.h?\n\n\n\nWe have utils.h of course, but that's in internal/ and this needs to be\nin public header space.\n\nOf course we could also have a public utils.h ... but I'm a bit opposed\nto having lots of duplicated header names in both public and private\nspace, as it gets confusing as to which one is being used.\n\n\n> \n>> Signed-off-by: Kieran Bingham \n>> ---\n>> include/libcamera/class.h | 35 +++++++++++++++++++++++++++++++++++\n>> include/libcamera/meson.build | 1 +\n>> 2 files changed, 36 insertions(+)\n>> create mode 100644 include/libcamera/class.h\n>>\n>> diff --git a/include/libcamera/class.h b/include/libcamera/class.h\n>> new file mode 100644\n>> index 000000000000..adcfa8860957\n>> --- /dev/null\n>> +++ b/include/libcamera/class.h\n>> @@ -0,0 +1,35 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2020, Google Inc.\n>> + *\n>> + * class.h - class helpers\n>> + */\n>> +#ifndef __LIBCAMERA_CLASS_H__\n>> +#define __LIBCAMERA_CLASS_H__\n>> +\n>> +/**\n>> + * \\brief Delete the copy constructor and assignment operator.\n> \n> Missing \\param ?\n> \n>> + */\n>> +#define DELETE_COPY_AND_ASSIGN(klass) \\\n> \n> Macros are unfortunately not part of namespaces, so to avoid a risk of\n> collision, I'd use a LIBCAMERA_ prefix. And to avoid the name getting\n> too long, I think you can drop the _AND_ASSIGN suffix, as the macro is\n> disabling copying as a whole. Otherwise it should be\n> DELETE_COPY_CONSTRUCT_AND_ASSIGN :-)\n> \n> Another bikeshedding topic, I would have gone for DISABLE instead of\n> DELETE to emphasize the purpose of the macro instead of how it's\n> implemented (not that libcamera has to care about this, but in older C++\n> versions we would have needed to declare the functions as private as\n> there was no = delete).\n\nThat's why I chose delete, to express explicitly that this is 'deleting'\nthe functions.\n\nThe similar 'google' macros are called 'DISALLOW_'...\n\nIn fact, somewhat frustratingly, the chromium style guides deprecated\nthe equivalent macros:\n\nhttps://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++-dos-and-donts.md\n\nAnd instead prefer to code them directly. But I disagree, as we've seen\nthis can lead to errors, both in not deleting both the constructor and\nassignment operator, or in getting the names wrong, and quite honestly I\nfind them really hard to interpret, just from looking at them. (More below).\n\n> \n>> +\t/* copy constructor. */ \\\n> \n> I think you can drop the comments, it's quite explicit already.\n\nThe reason I've put these here, is because I find it really hard to look\nat these and say \"Oh - that's the copy constructor\", (as opposed to the\nmove constructor), and \"That's the copy assignment operator\", as opposed\nto the move assignment operator.\n\nLets consider it like the C++ equivalent of English Homophones. Things\nthat look/read/sound similar but have a different meaning.\n\ni.e. : If I hadn't looked at this in more than one day, I would have to\nlook up which of these does which :\n\tklass(klass &&) = delete;\n\tklass(const klass &) = delete;\n\nand equally for the operators:\n\tklass &operator=(const klass &) = delete\n\tklass &operator=(klass &&) = delete\n\nAnd in code - I don't want the meaning of the code to be easy to\nmis-interpret.\n\n\n> \n>> +\tklass(const klass &) = delete; \\\n>> +\t/* copy assignment operator. */ \\\n>> +\tklass &operator=(const klass &) = delete\n>> +\n>> +/**\n>> + * \\brief Delete the move construtor and assignment operator.\n>> + */\n>> +#define DELETE_MOVE_AND_ASSIGN(klass) \\\n>> +\t/* move constructor. */ \\\n>> +\tklass(klass &&) = delete; \\\n>> +\t/* move assignment operator. */ \\\n>> +\tklass &operator=(klass &&) = delete\n>> +\n>> +/**\n>> + * \\brief Delete all copy and move constructors, and assignment operators.\n>> + */\n>> +#define DELETE_COPY_MOVE_AND_ASSIGN(klass) \\\n>> +\tDELETE_COPY_AND_ASSIGN(klass); \\\n>> +\tDELETE_MOVE_AND_ASSIGN(klass)\n>> +\n>> +#endif /* __LIBCAMERA_CLASS_H__ */\n>> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n>> index 3d5fc70134ad..b3363a6f735b 100644\n>> --- a/include/libcamera/meson.build\n>> +++ b/include/libcamera/meson.build\n>> @@ -5,6 +5,7 @@ libcamera_public_headers = files([\n>> 'buffer.h',\n>> 'camera.h',\n>> 'camera_manager.h',\n>> + 'class.h',\n>> 'controls.h',\n>> 'event_dispatcher.h',\n>> 'event_notifier.h',\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 7D24AC3D3C\n\tfor ;\n\tFri, 23 Oct 2020 08:17:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 00D25615D2;\n\tFri, 23 Oct 2020 10:17:01 +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 3013660350\n\tfor ;\n\tFri, 23 Oct 2020 10:16:59 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EE864B26;\n\tFri, 23 Oct 2020 10:16:57 +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=\"B7Q2ECnA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603441018;\n\tbh=xJtKxoRbsQmTlZU4OaxbN48Z8K6pBcavZIKxfGxcT1Q=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=B7Q2ECnAZO7Td23sTKztBFf0UKnkPe71mQBio38oIv+xVoSpViHyw0fldxGZEm9z0\n\tpEh60LWi2jGpp5NuBfWOyMgK8+9KlpuieifdrPGr+fKkFSClEwcyzMPwyoIcJzXYSg\n\tn42v6Ie07C3dmUtphbWGGUoi2TMtvwhdRXm+rOak=","To":"Laurent Pinchart ","References":"<20201022135605.614240-1-kieran.bingham@ideasonboard.com>\n\t<20201022135605.614240-2-kieran.bingham@ideasonboard.com>\n\t<20201023042306.GB4113@pendragon.ideasonboard.com>","From":"Kieran Bingham ","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\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<577f983a-4a3b-eee4-b29c-c5c1343abeb2@ideasonboard.com>","Date":"Fri, 23 Oct 2020 09:16:55 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201023042306.GB4113@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Provide class.h","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13451,"web_url":"https://patchwork.libcamera.org/comment/13451/","msgid":"<20201023201138.GC5979@pendragon.ideasonboard.com>","date":"2020-10-23T20:11:38","subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Provide class.h","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Fri, Oct 23, 2020 at 09:16:55AM +0100, Kieran Bingham wrote:\n> On 23/10/2020 05:23, Laurent Pinchart wrote:\n> > On Thu, Oct 22, 2020 at 02:56:01PM +0100, Kieran Bingham wrote:\n> >> Provide a public header to define helpers for class definitions.\n> >> This initially implements macros to clearly define the deletion of\n> >> copy/move/assignment constructors/operators for classes to restrict\n> >> their capabilities explicitly.\n> > \n> > Thanks for picking this up after my unsuccessful experimented with a\n> > non-copyable base class. There's a few bikesheeding comments beow, but\n> > idea looks good.\n> > \n> > Have you considered including this in a more generic header, such as\n> > utils.h (or global.h, or .h) ? I don't foresee lots of new\n> > features to be added to class.h, while I think we'll have more\n> > miscellaneous macros and functions moving forward. It would be nice to\n> > avoid creating micro-headers.\n> \n> That's why I chose class.h ...\n> \n> I thought this would cover more than just these macros.\n> \n> Throwing your argument back at you, You've recently posted a patch which\n> adds 'extensible.h' ... and both these, and your extensible concepts are\n> attributed to 'class' helpers, (yet these could not go in to\n> extensible.h) ...\n> \n> So ... Would you prefer to put your extensible types in class.h, or\n> global.h?\n> \n> We have utils.h of course, but that's in internal/ and this needs to be\n> in public header space.\n>\n> Of course we could also have a public utils.h ... but I'm a bit opposed\n> to having lots of duplicated header names in both public and private\n> space, as it gets confusing as to which one is being used.\n\nThe move of the private headers to an internal/ directory was meant to\nallow public and private headers with the same name, but it of course\ndoesn't mean we have to (ab)use that feature.\n\nMerging these macros and the Extensible class in a single header makes\nsense to me. That broadens the scope of class.h a bit so we could go\nwith that. I still feel we'll have a need for miscellaneous public\n\"things\" in the future which wouldn't be a good match for class.h, but\nwe could address that problem at that time.\n\n> >> Signed-off-by: Kieran Bingham \n> >> ---\n> >> include/libcamera/class.h | 35 +++++++++++++++++++++++++++++++++++\n> >> include/libcamera/meson.build | 1 +\n> >> 2 files changed, 36 insertions(+)\n> >> create mode 100644 include/libcamera/class.h\n> >>\n> >> diff --git a/include/libcamera/class.h b/include/libcamera/class.h\n> >> new file mode 100644\n> >> index 000000000000..adcfa8860957\n> >> --- /dev/null\n> >> +++ b/include/libcamera/class.h\n> >> @@ -0,0 +1,35 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2020, Google Inc.\n> >> + *\n> >> + * class.h - class helpers\n> >> + */\n> >> +#ifndef __LIBCAMERA_CLASS_H__\n> >> +#define __LIBCAMERA_CLASS_H__\n> >> +\n> >> +/**\n> >> + * \\brief Delete the copy constructor and assignment operator.\n> > \n> > Missing \\param ?\n> > \n> >> + */\n> >> +#define DELETE_COPY_AND_ASSIGN(klass) \\\n> > \n> > Macros are unfortunately not part of namespaces, so to avoid a risk of\n> > collision, I'd use a LIBCAMERA_ prefix. And to avoid the name getting\n> > too long, I think you can drop the _AND_ASSIGN suffix, as the macro is\n> > disabling copying as a whole. Otherwise it should be\n> > DELETE_COPY_CONSTRUCT_AND_ASSIGN :-)\n> > \n> > Another bikeshedding topic, I would have gone for DISABLE instead of\n> > DELETE to emphasize the purpose of the macro instead of how it's\n> > implemented (not that libcamera has to care about this, but in older C++\n> > versions we would have needed to declare the functions as private as\n> > there was no = delete).\n> \n> That's why I chose delete, to express explicitly that this is 'deleting'\n> the functions.\n> \n> The similar 'google' macros are called 'DISALLOW_'...\n\nAnd one more data point,\nhttps://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY :-)\n\nI prefer expressing the purpose in the name rather than the\nimplementation details. That arguments makes more sense for Qt that has\nto support multiple C++ versions, including older versions where =\ndelete wasn't available.\n\nIf you prefer DELETE over DISABLE I won't fight hard for it (we know we\nwant to avoid DISALLOW as that's what Google deprecates, but DISABLE or\nDELETE are not deprecated, right ? ;-)). I feel a bit stronger about\nwriting 'COPY' instead of 'COPY_AND_ASSIGN', as the copy concept is\nreally the combination of a constructor and an assignment operator (same\nfor move).\n\n> In fact, somewhat frustratingly, the chromium style guides deprecated\n> the equivalent macros:\n> \n> https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++-dos-and-donts.md\n> \n> And instead prefer to code them directly. But I disagree, as we've seen\n> this can lead to errors, both in not deleting both the constructor and\n> assignment operator, or in getting the names wrong, and quite honestly I\n> find them really hard to interpret, just from looking at them. (More below).\n\nI agree with you, I think the macros make sense. We wouldn't be having\nthis conversation if just deleting the correct functions wasn't\nerror-prone.\n\n> > \n> >> +\t/* copy constructor. */ \\\n> > \n> > I think you can drop the comments, it's quite explicit already.\n> \n> The reason I've put these here, is because I find it really hard to look\n> at these and say \"Oh - that's the copy constructor\", (as opposed to the\n> move constructor), and \"That's the copy assignment operator\", as opposed\n> to the move assignment operator.\n> \n> Lets consider it like the C++ equivalent of English Homophones. Things\n> that look/read/sound similar but have a different meaning.\n> \n> i.e. : If I hadn't looked at this in more than one day, I would have to\n> look up which of these does which :\n> \tklass(klass &&) = delete;\n> \tklass(const klass &) = delete;\n> \n> and equally for the operators:\n> \tklass &operator=(const klass &) = delete\n> \tklass &operator=(klass &&) = delete\n> \n> And in code - I don't want the meaning of the code to be easy to\n> mis-interpret.\n\nMaybe it's a matter of getting used to it :-) My comment was more about\nthe fact that telling the constructor and assignment operator apart is\nfairly straightforward (at least more than telling the copy and move\nversions apart at a quick glance), and which pair you delete is part of\nthe macro name. That's why I didn't consider the comments to add much,\nbut if you think they help, I don't mind (we usually avoid comments in\nheaders though, but there are exceptions where it makes sense).\n\n> >> +\tklass(const klass &) = delete; \\\n> >> +\t/* copy assignment operator. */ \\\n> >> +\tklass &operator=(const klass &) = delete\n> >> +\n> >> +/**\n> >> + * \\brief Delete the move construtor and assignment operator.\n> >> + */\n> >> +#define DELETE_MOVE_AND_ASSIGN(klass) \\\n> >> +\t/* move constructor. */ \\\n> >> +\tklass(klass &&) = delete; \\\n> >> +\t/* move assignment operator. */ \\\n> >> +\tklass &operator=(klass &&) = delete\n> >> +\n> >> +/**\n> >> + * \\brief Delete all copy and move constructors, and assignment operators.\n> >> + */\n> >> +#define DELETE_COPY_MOVE_AND_ASSIGN(klass) \\\n> >> +\tDELETE_COPY_AND_ASSIGN(klass); \\\n> >> +\tDELETE_MOVE_AND_ASSIGN(klass)\n> >> +\n> >> +#endif /* __LIBCAMERA_CLASS_H__ */\n> >> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> >> index 3d5fc70134ad..b3363a6f735b 100644\n> >> --- a/include/libcamera/meson.build\n> >> +++ b/include/libcamera/meson.build\n> >> @@ -5,6 +5,7 @@ libcamera_public_headers = files([\n> >> 'buffer.h',\n> >> 'camera.h',\n> >> 'camera_manager.h',\n> >> + 'class.h',\n> >> 'controls.h',\n> >> 'event_dispatcher.h',\n> >> 'event_notifier.h',","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 D207BBDB13\n\tfor ;\n\tFri, 23 Oct 2020 20:12:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 26518615D3;\n\tFri, 23 Oct 2020 22:12:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5F6B160350\n\tfor ;\n\tFri, 23 Oct 2020 22:12:25 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B9C8DB26;\n\tFri, 23 Oct 2020 22:12:24 +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=\"eFR0RLqJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603483944;\n\tbh=5FM/2A9wvDCfU5DJn7gT3lPCbyMNzNI0+gbMoYzeCQI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eFR0RLqJEX0rqndm2XnP/Sm3XdrrPaAXC7Sw/YdkHeVcI6VqoFMzzRMa4EB2DisVQ\n\tdUm3JFyLmgNVe/Fa4Ec08JvHgsDMGXIL4e//v6drtfxs0rqhk/xLoxlECscU8qCbDI\n\tN7bHm+woUX4IhvVMTOR0IxL7IUJ6Uxi+2bhLH0l4=","Date":"Fri, 23 Oct 2020 23:11:38 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20201023201138.GC5979@pendragon.ideasonboard.com>","References":"<20201022135605.614240-1-kieran.bingham@ideasonboard.com>\n\t<20201022135605.614240-2-kieran.bingham@ideasonboard.com>\n\t<20201023042306.GB4113@pendragon.ideasonboard.com>\n\t<577f983a-4a3b-eee4-b29c-c5c1343abeb2@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<577f983a-4a3b-eee4-b29c-c5c1343abeb2@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Provide class.h","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 ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15049,"web_url":"https://patchwork.libcamera.org/comment/15049/","msgid":"","date":"2021-02-08T11:37:43","subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Provide class.h","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 23/10/2020 21:11, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Fri, Oct 23, 2020 at 09:16:55AM +0100, Kieran Bingham wrote:\n>> On 23/10/2020 05:23, Laurent Pinchart wrote:\n>>> On Thu, Oct 22, 2020 at 02:56:01PM +0100, Kieran Bingham wrote:\n>>>> Provide a public header to define helpers for class definitions.\n>>>> This initially implements macros to clearly define the deletion of\n>>>> copy/move/assignment constructors/operators for classes to restrict\n>>>> their capabilities explicitly.\n>>>\n>>> Thanks for picking this up after my unsuccessful experimented with a\n>>> non-copyable base class. There's a few bikesheeding comments beow, but\n>>> idea looks good.\n>>>\n>>> Have you considered including this in a more generic header, such as\n>>> utils.h (or global.h, or .h) ? I don't foresee lots of new\n>>> features to be added to class.h, while I think we'll have more\n>>> miscellaneous macros and functions moving forward. It would be nice to\n>>> avoid creating micro-headers.\n>>\n>> That's why I chose class.h ...\n>>\n>> I thought this would cover more than just these macros.\n>>\n>> Throwing your argument back at you, You've recently posted a patch which\n>> adds 'extensible.h' ... and both these, and your extensible concepts are\n>> attributed to 'class' helpers, (yet these could not go in to\n>> extensible.h) ...\n>>\n>> So ... Would you prefer to put your extensible types in class.h, or\n>> global.h?\n>>\n>> We have utils.h of course, but that's in internal/ and this needs to be\n>> in public header space.\n>>\n>> Of course we could also have a public utils.h ... but I'm a bit opposed\n>> to having lots of duplicated header names in both public and private\n>> space, as it gets confusing as to which one is being used.\n> \n> The move of the private headers to an internal/ directory was meant to\n> allow public and private headers with the same name, but it of course\n> doesn't mean we have to (ab)use that feature.\n> \n> Merging these macros and the Extensible class in a single header makes\n> sense to me. That broadens the scope of class.h a bit so we could go\n> with that. I still feel we'll have a need for miscellaneous public\n> \"things\" in the future which wouldn't be a good match for class.h, but\n> we could address that problem at that time.\n\nNow that you have merged the Extensible.h header, do you have any\npreference for where these helpers would live?\n\n\n>>>> Signed-off-by: Kieran Bingham \n>>>> ---\n>>>> include/libcamera/class.h | 35 +++++++++++++++++++++++++++++++++++\n>>>> include/libcamera/meson.build | 1 +\n>>>> 2 files changed, 36 insertions(+)\n>>>> create mode 100644 include/libcamera/class.h\n>>>>\n>>>> diff --git a/include/libcamera/class.h b/include/libcamera/class.h\n>>>> new file mode 100644\n>>>> index 000000000000..adcfa8860957\n>>>> --- /dev/null\n>>>> +++ b/include/libcamera/class.h\n>>>> @@ -0,0 +1,35 @@\n>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>> +/*\n>>>> + * Copyright (C) 2020, Google Inc.\n>>>> + *\n>>>> + * class.h - class helpers\n>>>> + */\n>>>> +#ifndef __LIBCAMERA_CLASS_H__\n>>>> +#define __LIBCAMERA_CLASS_H__\n>>>> +\n>>>> +/**\n>>>> + * \\brief Delete the copy constructor and assignment operator.\n>>>\n>>> Missing \\param ?\n>>>\n\nHow about:\n\n+ * \\param klass The identifier of the class to modify\n\n\n>>>> + */\n>>>> +#define DELETE_COPY_AND_ASSIGN(klass) \\\n>>>\n>>> Macros are unfortunately not part of namespaces, so to avoid a risk of\n>>> collision, I'd use a LIBCAMERA_ prefix. And to avoid the name getting\n>>> too long, I think you can drop the _AND_ASSIGN suffix, as the macro is\n>>> disabling copying as a whole. Otherwise it should be\n>>> DELETE_COPY_CONSTRUCT_AND_ASSIGN :-)\n\nLIBCAMERA_ feels a bit extraneous to me ... but I can add it.\n(We could also wrap it in a #ifndef DELETE_COPY_xxxxx)\n\nDELETE_COPY_CONSTRUCT_AND_ASSIGN is really quite long...\nand\nLIBCAMERA_DELETE_COPY_CONSTRUCT_AND_ASSIGN\neven more so.\n\n>>>\n>>> Another bikeshedding topic, I would have gone for DISABLE instead of\n>>> DELETE to emphasize the purpose of the macro instead of how it's\n>>> implemented (not that libcamera has to care about this, but in older C++\n>>> versions we would have needed to declare the functions as private as\n>>> there was no = delete).\n>>\n>> That's why I chose delete, to express explicitly that this is 'deleting'\n>> the functions.\n>>\n>> The similar 'google' macros are called 'DISALLOW_'...\n> \n> And one more data point,\n> https://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY :-)\n> \n> I prefer expressing the purpose in the name rather than the\n> implementation details. That arguments makes more sense for Qt that has\n> to support multiple C++ versions, including older versions where =\n> delete wasn't available.\n\nI still prefer delete ...\n\nThese macros are providing syntactic sugar to make sure 'what' is being\ndeleted is explicit (in human readable terms).\n\nAnd I don't believe we'll be supporting a C++ version which can't handle\n= delete ?\n\n\n> If you prefer DELETE over DISABLE I won't fight hard for it (we know we\n> want to avoid DISALLOW as that's what Google deprecates, but DISABLE or\n> DELETE are not deprecated, right ? ;-)). I feel a bit stronger about\n> writing 'COPY' instead of 'COPY_AND_ASSIGN', as the copy concept is\n> really the combination of a constructor and an assignment operator (same\n> for move).\n\nTell me what colour you'd like this and I'll buy the paint.\n\nWould you like to just copy QT and go with\n\nLIBCAMERA_DISABLE_COPY\nLIBCAMERA_DISABLE_MOVE?\n\n\n>> In fact, somewhat frustratingly, the chromium style guides deprecated\n>> the equivalent macros:\n>>\n>> https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++-dos-and-donts.md\n>>\n>> And instead prefer to code them directly. But I disagree, as we've seen\n>> this can lead to errors, both in not deleting both the constructor and\n>> assignment operator, or in getting the names wrong, and quite honestly I\n>> find them really hard to interpret, just from looking at them. (More below).\n> \n> I agree with you, I think the macros make sense. We wouldn't be having\n> this conversation if just deleting the correct functions wasn't\n> error-prone.\n> \n>>>\n>>>> +\t/* copy constructor. */ \\\n>>>\n>>> I think you can drop the comments, it's quite explicit already.\n>>\n>> The reason I've put these here, is because I find it really hard to look\n>> at these and say \"Oh - that's the copy constructor\", (as opposed to the\n>> move constructor), and \"That's the copy assignment operator\", as opposed\n>> to the move assignment operator.\n>>\n>> Lets consider it like the C++ equivalent of English Homophones. Things\n>> that look/read/sound similar but have a different meaning.\n>>\n>> i.e. : If I hadn't looked at this in more than one day, I would have to\n>> look up which of these does which :\n>> \tklass(klass &&) = delete;\n>> \tklass(const klass &) = delete;\n\n\nWow - coming back at this, and I really have to look up which version of\neach does what. Which only solidifies in my head that these would be\nmuch better with names.\n\n\n>>\n>> and equally for the operators:\n>> \tklass &operator=(const klass &) = delete\n>> \tklass &operator=(klass &&) = delete\n>>\n>> And in code - I don't want the meaning of the code to be easy to\n>> mis-interpret.\n> \n> Maybe it's a matter of getting used to it :-) My comment was more about\n\nPerhaps someone who writes move/copy constructors everyday would easily\nremember the declarations and it would become as obvious as reading a\nfor-loop... but they're a low churn code type, and often copied as a\ntemplate anyway.\n\n\n> the fact that telling the constructor and assignment operator apart is\n> fairly straightforward (at least more than telling the copy and move\n> versions apart at a quick glance), and which pair you delete is part of\n> the macro name. That's why I didn't consider the comments to add much,\n> but if you think they help, I don't mind (we usually avoid comments in\n> headers though, but there are exceptions where it makes sense).\n> \n>>>> +\tklass(const klass &) = delete; \\\n>>>> +\t/* copy assignment operator. */ \\\n>>>> +\tklass &operator=(const klass &) = delete\n>>>> +\n>>>> +/**\n>>>> + * \\brief Delete the move construtor and assignment operator.\n>>>> + */\n>>>> +#define DELETE_MOVE_AND_ASSIGN(klass) \\\n>>>> +\t/* move constructor. */ \\\n>>>> +\tklass(klass &&) = delete; \\\n>>>> +\t/* move assignment operator. */ \\\n>>>> +\tklass &operator=(klass &&) = delete\n>>>> +\n>>>> +/**\n>>>> + * \\brief Delete all copy and move constructors, and assignment operators.\n>>>> + */\n>>>> +#define DELETE_COPY_MOVE_AND_ASSIGN(klass) \\\n>>>> +\tDELETE_COPY_AND_ASSIGN(klass); \\\n>>>> +\tDELETE_MOVE_AND_ASSIGN(klass)\n>>>> +\n>>>> +#endif /* __LIBCAMERA_CLASS_H__ */\n>>>> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n>>>> index 3d5fc70134ad..b3363a6f735b 100644\n>>>> --- a/include/libcamera/meson.build\n>>>> +++ b/include/libcamera/meson.build\n>>>> @@ -5,6 +5,7 @@ libcamera_public_headers = files([\n>>>> 'buffer.h',\n>>>> 'camera.h',\n>>>> 'camera_manager.h',\n>>>> + 'class.h',\n>>>> 'controls.h',\n>>>> 'event_dispatcher.h',\n>>>> 'event_notifier.h',\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 01AF3BD160\n\tfor ;\n\tMon, 8 Feb 2021 11:37:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C236760D18;\n\tMon, 8 Feb 2021 12:37:47 +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 090A660D18\n\tfor ;\n\tMon, 8 Feb 2021 12:37:46 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9CABF9FF;\n\tMon, 8 Feb 2021 12:37:45 +0100 (CET)"],"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=\"NuCn3ZF5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612784265;\n\tbh=y6xyORQElwyGVtyLYfMZZ1BEqJXsEJ1QMG8+MjZT/So=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=NuCn3ZF5s/fnmRANuESBx076cmXUxdhzG0VIa7QVhXpyw4TPPY6QKWNW8mTd903i5\n\tCvxug7zkITlF9LQxj1k/aS3+dSTamxhrSKb8BkUF+SG3PXJVxV5MNT7nuPkM0+7Pei\n\tfuc/XUhIpjCEV1yfvnP43edO4AnIqh5CHJyhbtoc=","To":"Laurent Pinchart ","References":"<20201022135605.614240-1-kieran.bingham@ideasonboard.com>\n\t<20201022135605.614240-2-kieran.bingham@ideasonboard.com>\n\t<20201023042306.GB4113@pendragon.ideasonboard.com>\n\t<577f983a-4a3b-eee4-b29c-c5c1343abeb2@ideasonboard.com>\n\t<20201023201138.GC5979@pendragon.ideasonboard.com>","From":"Kieran Bingham ","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\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Mon, 8 Feb 2021 11:37:43 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201023201138.GC5979@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 1/5] libcamera: Provide class.h","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]