[{"id":13425,"web_url":"https://patchwork.libcamera.org/comment/13425/","msgid":"<20201023042914.GD4113@pendragon.ideasonboard.com>","date":"2020-10-23T04:29:14","subject":"Re: [libcamera-devel] [PATCH 3/5] libcamera: media_object: Utilise\n\tDELETE_COPY_AND_ASSIGN","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:03PM +0100, Kieran Bingham wrote:\n> Convert MediaLink, MediaPad, and MediaEntity to declare DELETE_COPY_AND_ASSIGN.\n> These classes already deleted their copy constructor but not the assignment operator.\n> \n> Utilising the DELETE_COPY_AND_ASSIGN prevents all copying of these classes.\n\nLine wrap ?\n\n> Signed-off-by: Kieran Bingham \n> ---\n> include/libcamera/internal/media_object.h | 8 +++++---\n> 1 file changed, 5 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h\n> index be6fb8961349..f467883072d2 100644\n> --- a/include/libcamera/internal/media_object.h\n> +++ b/include/libcamera/internal/media_object.h\n> @@ -12,6 +12,8 @@\n> \n> #include \n> \n> +#include \n> +\n> namespace libcamera {\n> \n> class MediaDevice;\n> @@ -50,7 +52,7 @@ private:\n> \n> \tMediaLink(const struct media_v2_link *link,\n> \t\t MediaPad *source, MediaPad *sink);\n> -\tMediaLink(const MediaLink &) = delete;\n> +\tDELETE_COPY_AND_ASSIGN(MediaLink);\n\nYou have placed DELETE_COPY_AND_ASSIGN() right after private: in patch\n2/5. We could try and use the same placement in all classes for\nconsistency. I don't mind much.\n\nReviewed-by: Laurent Pinchart \n\n> \t~MediaLink() {}\n> \n> \tMediaPad *source_;\n> @@ -72,7 +74,7 @@ private:\n> \tfriend class MediaDevice;\n> \n> \tMediaPad(const struct media_v2_pad *pad, MediaEntity *entity);\n> -\tMediaPad(const MediaPad &) = delete;\n> +\tDELETE_COPY_AND_ASSIGN(MediaPad);\n> \t~MediaPad();\n> \n> \tunsigned int index_;\n> @@ -104,7 +106,7 @@ private:\n> \n> \tMediaEntity(MediaDevice *dev, const struct media_v2_entity *entity,\n> \t\t unsigned int major = 0, unsigned int minor = 0);\n> -\tMediaEntity(const MediaEntity &) = delete;\n> +\tDELETE_COPY_AND_ASSIGN(MediaEntity);\n> \t~MediaEntity();\n> \n> \tvoid addPad(MediaPad *pad);","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 7E5DBC3D3C\n\tfor ;\n\tFri, 23 Oct 2020 04:30:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DAF95615D6;\n\tFri, 23 Oct 2020 06:30:01 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 008236034E\n\tfor ;\n\tFri, 23 Oct 2020 06:30:00 +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 672CC53;\n\tFri, 23 Oct 2020 06:30:00 +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=\"TtoExUfG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603427400;\n\tbh=Bjn5AyOuCD7YBYDBwN7KQqBYH5qutgC5xl/zjf7Ah+o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TtoExUfGxOU9Mzzcf73CdV6CWvyRRJ/O5IvK8AegiHG7T/n1L+njAHEE6Xwvzxecy\n\t2JjSQBJROvCV4AdnfmdBzIa57Nzgd8Nrg9orA0mYOh6fEt9mn3UlC4MsBymR7IFzDn\n\tbhRkw39f3z0wZaCqMH1KDNQ1nnOxaOQPqixRormg=","Date":"Fri, 23 Oct 2020 07:29:14 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20201023042914.GD4113@pendragon.ideasonboard.com>","References":"<20201022135605.614240-1-kieran.bingham@ideasonboard.com>\n\t<20201022135605.614240-4-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201022135605.614240-4-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 3/5] libcamera: media_object: Utilise\n\tDELETE_COPY_AND_ASSIGN","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":13426,"web_url":"https://patchwork.libcamera.org/comment/13426/","msgid":"<20201023043304.GE4113@pendragon.ideasonboard.com>","date":"2020-10-23T04:33:04","subject":"Re: [libcamera-devel] [PATCH 3/5] libcamera: media_object: Utilise\n\tDELETE_COPY_AND_ASSIGN","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Oct 23, 2020 at 07:29:14AM +0300, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Thu, Oct 22, 2020 at 02:56:03PM +0100, Kieran Bingham wrote:\n> > Convert MediaLink, MediaPad, and MediaEntity to declare DELETE_COPY_AND_ASSIGN.\n> > These classes already deleted their copy constructor but not the assignment operator.\n> > \n> > Utilising the DELETE_COPY_AND_ASSIGN prevents all copying of these classes.\n> \n> Line wrap ?\n> \n> > Signed-off-by: Kieran Bingham \n> > ---\n> > include/libcamera/internal/media_object.h | 8 +++++---\n> > 1 file changed, 5 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h\n> > index be6fb8961349..f467883072d2 100644\n> > --- a/include/libcamera/internal/media_object.h\n> > +++ b/include/libcamera/internal/media_object.h\n> > @@ -12,6 +12,8 @@\n> > \n> > #include \n> > \n> > +#include \n> > +\n> > namespace libcamera {\n> > \n> > class MediaDevice;\n> > @@ -50,7 +52,7 @@ private:\n> > \n> > \tMediaLink(const struct media_v2_link *link,\n> > \t\t MediaPad *source, MediaPad *sink);\n> > -\tMediaLink(const MediaLink &) = delete;\n> > +\tDELETE_COPY_AND_ASSIGN(MediaLink);\n> \n> You have placed DELETE_COPY_AND_ASSIGN() right after private: in patch\n> 2/5. We could try and use the same placement in all classes for\n> consistency. I don't mind much.\n> \n> Reviewed-by: Laurent Pinchart \n\nActually I think we should disable both copy and move operations,\nthere's no need to move these classes. This is known as the rules of\nfive: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-copy-move-or-destructor-function-define-or-delete-them-all\n\n> > \t~MediaLink() {}\n> > \n> > \tMediaPad *source_;\n> > @@ -72,7 +74,7 @@ private:\n> > \tfriend class MediaDevice;\n> > \n> > \tMediaPad(const struct media_v2_pad *pad, MediaEntity *entity);\n> > -\tMediaPad(const MediaPad &) = delete;\n> > +\tDELETE_COPY_AND_ASSIGN(MediaPad);\n> > \t~MediaPad();\n> > \n> > \tunsigned int index_;\n> > @@ -104,7 +106,7 @@ private:\n> > \n> > \tMediaEntity(MediaDevice *dev, const struct media_v2_entity *entity,\n> > \t\t unsigned int major = 0, unsigned int minor = 0);\n> > -\tMediaEntity(const MediaEntity &) = delete;\n> > +\tDELETE_COPY_AND_ASSIGN(MediaEntity);\n> > \t~MediaEntity();\n> > \n> > \tvoid addPad(MediaPad *pad);","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 5BF1CBDB13\n\tfor ;\n\tFri, 23 Oct 2020 04:33:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE129615D4;\n\tFri, 23 Oct 2020 06:33:51 +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 C088B6034E\n\tfor ;\n\tFri, 23 Oct 2020 06:33:50 +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 2A0FA53;\n\tFri, 23 Oct 2020 06:33:50 +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=\"Nleco/T6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603427630;\n\tbh=IkA1PtKkuYj/ruGy61V7VlJFBRX2RWySAwUWf/kS/RM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Nleco/T6tN9+cwXbTMQh59dpH1MOXmieG0QfV+uOwBO2sA8bwGfVgetGWTn15phwq\n\tkQFhboOslXBlLPQGfqrsclxXvwWYuTSvO37GFqNrKTWzaImhjLHL87R9WlexEc78Qq\n\twe45tbPgYH0f72RKK/YH491B8Pk9LOlOq4zDTFmM=","Date":"Fri, 23 Oct 2020 07:33:04 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20201023043304.GE4113@pendragon.ideasonboard.com>","References":"<20201022135605.614240-1-kieran.bingham@ideasonboard.com>\n\t<20201022135605.614240-4-kieran.bingham@ideasonboard.com>\n\t<20201023042914.GD4113@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201023042914.GD4113@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 3/5] libcamera: media_object: Utilise\n\tDELETE_COPY_AND_ASSIGN","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":13436,"web_url":"https://patchwork.libcamera.org/comment/13436/","msgid":"","date":"2020-10-23T08:27:10","subject":"Re: [libcamera-devel] [PATCH 3/5] libcamera: media_object: Utilise\n\tDELETE_COPY_AND_ASSIGN","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:33, Laurent Pinchart wrote:\n> On Fri, Oct 23, 2020 at 07:29:14AM +0300, Laurent Pinchart wrote:\n>> Hi Kieran,\n>>\n>> Thank you for the patch.\n>>\n>> On Thu, Oct 22, 2020 at 02:56:03PM +0100, Kieran Bingham wrote:\n>>> Convert MediaLink, MediaPad, and MediaEntity to declare DELETE_COPY_AND_ASSIGN.\n>>> These classes already deleted their copy constructor but not the assignment operator.\n>>>\n>>> Utilising the DELETE_COPY_AND_ASSIGN prevents all copying of these classes.\n>>\n>> Line wrap ?\n>>\n>>> Signed-off-by: Kieran Bingham \n>>> ---\n>>> include/libcamera/internal/media_object.h | 8 +++++---\n>>> 1 file changed, 5 insertions(+), 3 deletions(-)\n>>>\n>>> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h\n>>> index be6fb8961349..f467883072d2 100644\n>>> --- a/include/libcamera/internal/media_object.h\n>>> +++ b/include/libcamera/internal/media_object.h\n>>> @@ -12,6 +12,8 @@\n>>> \n>>> #include \n>>> \n>>> +#include \n>>> +\n>>> namespace libcamera {\n>>> \n>>> class MediaDevice;\n>>> @@ -50,7 +52,7 @@ private:\n>>> \n>>> \tMediaLink(const struct media_v2_link *link,\n>>> \t\t MediaPad *source, MediaPad *sink);\n>>> -\tMediaLink(const MediaLink &) = delete;\n>>> +\tDELETE_COPY_AND_ASSIGN(MediaLink);\n>>\n>> You have placed DELETE_COPY_AND_ASSIGN() right after private: in patch\n>> 2/5. We could try and use the same placement in all classes for\n>> consistency. I don't mind much.\n\nI think in those cases, there were no constructors also declared private.\n\nBut I don't mind keeping the DELETE_... consistent.\n\n\n>>\n>> Reviewed-by: Laurent Pinchart \n> \n> Actually I think we should disable both copy and move operations,\n> there's no need to move these classes. This is known as the rules of\n> five: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-copy-move-or-destructor-function-define-or-delete-them-all\n\nI was trying not to change code semantics in this series, though this\npatch is a bit of an exception, because it was only deleting the copy\nconstructor, without the copy assignment operator, and I considered that\na specific bug.\n\nOtherwise, by that rule, we don't need DELETE_COPY_AND_ASSIGN, and we\ncan always use DELETE_COPY_AND_MOVE in all of the other cases too.\n\nBut that's most certainly more of a functional change, so I'd like to do\nso on top, as it will require considering each class independently.\n\n\n\n\n>>> \t~MediaLink() {}\n>>> \n>>> \tMediaPad *source_;\n>>> @@ -72,7 +74,7 @@ private:\n>>> \tfriend class MediaDevice;\n>>> \n>>> \tMediaPad(const struct media_v2_pad *pad, MediaEntity *entity);\n>>> -\tMediaPad(const MediaPad &) = delete;\n>>> +\tDELETE_COPY_AND_ASSIGN(MediaPad);\n>>> \t~MediaPad();\n>>> \n>>> \tunsigned int index_;\n>>> @@ -104,7 +106,7 @@ private:\n>>> \n>>> \tMediaEntity(MediaDevice *dev, const struct media_v2_entity *entity,\n>>> \t\t unsigned int major = 0, unsigned int minor = 0);\n>>> -\tMediaEntity(const MediaEntity &) = delete;\n>>> +\tDELETE_COPY_AND_ASSIGN(MediaEntity);\n>>> \t~MediaEntity();\n>>> \n>>> \tvoid addPad(MediaPad *pad);\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 AA575BDB13\n\tfor ;\n\tFri, 23 Oct 2020 08:27:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1DA79615D4;\n\tFri, 23 Oct 2020 10:27:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 20F4260350\n\tfor ;\n\tFri, 23 Oct 2020 10:27:14 +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 9238BB26;\n\tFri, 23 Oct 2020 10:27:13 +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=\"IEjDUx3Z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603441633;\n\tbh=Ihfycxt9mdtyk1104aYPsrjKmS3VVVWG9oEfZlOcQ5E=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=IEjDUx3Z3U4Wg4vWMEut5bhYIt8hwgt8tgEuyOwFpjW3rxHEpBQX9uKVpp3Obzz9B\n\twUMVrJ/Fx09tguGfzKRVoMGLAY1Mp8q3d+uAAtD8tHMLBcY07AuI+AdxzpWlv7/qKM\n\tv+VW1gLpkFCC8GZy9KGiA7glcN5as1LM0yyf/Fcc=","To":"Laurent Pinchart ","References":"<20201022135605.614240-1-kieran.bingham@ideasonboard.com>\n\t<20201022135605.614240-4-kieran.bingham@ideasonboard.com>\n\t<20201023042914.GD4113@pendragon.ideasonboard.com>\n\t<20201023043304.GE4113@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":"Fri, 23 Oct 2020 09:27:10 +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":"<20201023043304.GE4113@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 3/5] libcamera: media_object: Utilise\n\tDELETE_COPY_AND_ASSIGN","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":13448,"web_url":"https://patchwork.libcamera.org/comment/13448/","msgid":"<20201023182615.GM4113@pendragon.ideasonboard.com>","date":"2020-10-23T18:26:15","subject":"Re: [libcamera-devel] [PATCH 3/5] libcamera: media_object: Utilise\n\tDELETE_COPY_AND_ASSIGN","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:27:10AM +0100, Kieran Bingham wrote:\n> On 23/10/2020 05:33, Laurent Pinchart wrote:\n> > On Fri, Oct 23, 2020 at 07:29:14AM +0300, Laurent Pinchart wrote:\n> >> On Thu, Oct 22, 2020 at 02:56:03PM +0100, Kieran Bingham wrote:\n> >>> Convert MediaLink, MediaPad, and MediaEntity to declare DELETE_COPY_AND_ASSIGN.\n> >>> These classes already deleted their copy constructor but not the assignment operator.\n> >>>\n> >>> Utilising the DELETE_COPY_AND_ASSIGN prevents all copying of these classes.\n> >>\n> >> Line wrap ?\n> >>\n> >>> Signed-off-by: Kieran Bingham \n> >>> ---\n> >>> include/libcamera/internal/media_object.h | 8 +++++---\n> >>> 1 file changed, 5 insertions(+), 3 deletions(-)\n> >>>\n> >>> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h\n> >>> index be6fb8961349..f467883072d2 100644\n> >>> --- a/include/libcamera/internal/media_object.h\n> >>> +++ b/include/libcamera/internal/media_object.h\n> >>> @@ -12,6 +12,8 @@\n> >>> \n> >>> #include \n> >>> \n> >>> +#include \n> >>> +\n> >>> namespace libcamera {\n> >>> \n> >>> class MediaDevice;\n> >>> @@ -50,7 +52,7 @@ private:\n> >>> \n> >>> \tMediaLink(const struct media_v2_link *link,\n> >>> \t\t MediaPad *source, MediaPad *sink);\n> >>> -\tMediaLink(const MediaLink &) = delete;\n> >>> +\tDELETE_COPY_AND_ASSIGN(MediaLink);\n> >>\n> >> You have placed DELETE_COPY_AND_ASSIGN() right after private: in patch\n> >> 2/5. We could try and use the same placement in all classes for\n> >> consistency. I don't mind much.\n> \n> I think in those cases, there were no constructors also declared private.\n> \n> But I don't mind keeping the DELETE_... consistent.\n> \n> >>\n> >> Reviewed-by: Laurent Pinchart \n> > \n> > Actually I think we should disable both copy and move operations,\n> > there's no need to move these classes. This is known as the rules of\n> > five: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-copy-move-or-destructor-function-define-or-delete-them-all\n> \n> I was trying not to change code semantics in this series, though this\n> patch is a bit of an exception, because it was only deleting the copy\n> constructor, without the copy assignment operator, and I considered that\n> a specific bug.\n> \n> Otherwise, by that rule, we don't need DELETE_COPY_AND_ASSIGN, and we\n> can always use DELETE_COPY_AND_MOVE in all of the other cases too.\n\nNote that the rule states that all constructors and operators should be\ndeclared, not that they all have to be deleted or all explicitly\nimplemented (unless I misunderstand something). I expect the most common\ncase to be disabling both copy and move, but there will be some cases\nwhere copy will be disabled and move explicitly defined\n(std::unique_ptr<> being the best example for that, it manages a unique\nresource, thus disabling copying, but allows moving).\n\n> But that's most certainly more of a functional change, so I'd like to do\n> so on top, as it will require considering each class independently.\n\nI'm fine doing so on top. This series is a good candidate though, as if\nwe delay it too much we'll forget about it :-)\n\n> >>> \t~MediaLink() {}\n> >>> \n> >>> \tMediaPad *source_;\n> >>> @@ -72,7 +74,7 @@ private:\n> >>> \tfriend class MediaDevice;\n> >>> \n> >>> \tMediaPad(const struct media_v2_pad *pad, MediaEntity *entity);\n> >>> -\tMediaPad(const MediaPad &) = delete;\n> >>> +\tDELETE_COPY_AND_ASSIGN(MediaPad);\n> >>> \t~MediaPad();\n> >>> \n> >>> \tunsigned int index_;\n> >>> @@ -104,7 +106,7 @@ private:\n> >>> \n> >>> \tMediaEntity(MediaDevice *dev, const struct media_v2_entity *entity,\n> >>> \t\t unsigned int major = 0, unsigned int minor = 0);\n> >>> -\tMediaEntity(const MediaEntity &) = delete;\n> >>> +\tDELETE_COPY_AND_ASSIGN(MediaEntity);\n> >>> \t~MediaEntity();\n> >>> \n> >>> \tvoid addPad(MediaPad *pad);","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 EF7A3C3D3C\n\tfor ;\n\tFri, 23 Oct 2020 18:27:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7CCD161417;\n\tFri, 23 Oct 2020 20:27:04 +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 5EC4860350\n\tfor ;\n\tFri, 23 Oct 2020 20:27:02 +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 C3AD0B26;\n\tFri, 23 Oct 2020 20:27:01 +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=\"a+9Prizu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603477622;\n\tbh=FyB0vXG0OMH24qwUN2bFF/+u8T4ASYmIMoTwtsvzDqw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=a+9PrizukGL2N4+mdOTfCHwLQ0pS22sOo27JdMRkqbw7yCTuk847vg+vG1+8Mxrnh\n\tRr1esuzyZl4mDkR0AaNO6D4cdQstSOt2mkvhOO0w24E+cXmlARMPaHJlpFz7zbYxxN\n\tWY69HrvlMDPZV87blaqbjN4irvj5xSeRXgYsqaSI=","Date":"Fri, 23 Oct 2020 21:26:15 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20201023182615.GM4113@pendragon.ideasonboard.com>","References":"<20201022135605.614240-1-kieran.bingham@ideasonboard.com>\n\t<20201022135605.614240-4-kieran.bingham@ideasonboard.com>\n\t<20201023042914.GD4113@pendragon.ideasonboard.com>\n\t<20201023043304.GE4113@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 3/5] libcamera: media_object: Utilise\n\tDELETE_COPY_AND_ASSIGN","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\" "}}]