[{"id":28913,"web_url":"https://patchwork.libcamera.org/comment/28913/","msgid":"<171016646589.252503.3525850997711483836@ping.linuxembedded.co.uk>","date":"2024-03-11T14:14:25","subject":"Re: [PATCH v2 1/3] libcamera: framebuffer_allocator: Move from\n\targument in constructor","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2024-03-10 14:30:33)\n> The single argument, of type `std::shared_ptr<Camera>`,\n> is passed by value, so it can simply be moved from in order to\n> avoid calling the copy constructor.\n> \n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> ---\n>  src/libcamera/framebuffer_allocator.cpp | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\n> index 94389735..8cf45ab2 100644\n> --- a/src/libcamera/framebuffer_allocator.cpp\n> +++ b/src/libcamera/framebuffer_allocator.cpp\n> @@ -59,7 +59,7 @@ LOG_DEFINE_CATEGORY(Allocator)\n>   * \\param[in] camera The camera\n>   */\n>  FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr<Camera> camera)\n> -       : camera_(camera)\n> +       : camera_(std::move(camera))\n>  {\n>  }\n>  \n> -- \n> 2.44.0\n> \n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 6740DBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 11 Mar 2024 14:14:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CACEE62868;\n\tMon, 11 Mar 2024 15:14:30 +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 39ADC61C7C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 11 Mar 2024 15:14:29 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 02BA7F0A;\n\tMon, 11 Mar 2024 15:14:07 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VOgqqr6t\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1710166448;\n\tbh=eVuJku+sgN2GvPaBMqXKV3ggFLWI+f4eHiDvAYd2q6A=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=VOgqqr6tXQs2hNoizERZ1yFXas4A+DtKK1sgIer8ERWFCDnGIPH/CmM3akQnzjSiS\n\te4pGvZ4ULi+kZCyN/35IjYvX3OG3BS06zN3ZH7DHX2KUq6azKozwcu5XNFXPb6KAU0\n\t7ajCe4NgwPvtZ/0gDfEcdsFXJPWS4Cy1n9m3RFLI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240310143023.752559-2-pobrn@protonmail.com>","References":"<20240310143023.752559-1-pobrn@protonmail.com>\n\t<20240310143023.752559-2-pobrn@protonmail.com>","Subject":"Re: [PATCH v2 1/3] libcamera: framebuffer_allocator: Move from\n\targument in constructor","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 11 Mar 2024 14:14:25 +0000","Message-ID":"<171016646589.252503.3525850997711483836@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28920,"web_url":"https://patchwork.libcamera.org/comment/28920/","msgid":"<w3k2jud53xfxktv3omg26ff3gi435lhur6ldtdqfkqe2dqjht4@dbqiahab4ciy>","date":"2024-03-11T17:01:15","subject":"Re: [PATCH v2 1/3] libcamera: framebuffer_allocator: Move from\n\targument in constructor","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Sun, Mar 10, 2024 at 02:30:33PM +0000, Barnabás Pőcze wrote:\n> The single argument, of type `std::shared_ptr<Camera>`,\n> is passed by value, so it can simply be moved from in order to\n> avoid calling the copy constructor.\n>\n> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> ---\n>  src/libcamera/framebuffer_allocator.cpp | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\n> index 94389735..8cf45ab2 100644\n> --- a/src/libcamera/framebuffer_allocator.cpp\n> +++ b/src/libcamera/framebuffer_allocator.cpp\n> @@ -59,7 +59,7 @@ LOG_DEFINE_CATEGORY(Allocator)\n>   * \\param[in] camera The camera\n>   */\n>  FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr<Camera> camera)\n> -\t: camera_(camera)\n> +\t: camera_(std::move(camera))\n\nHowever, if the caller of the FrameBufferAllocator constructor uses\nmove() when calling the constructur, this will not increase the\nshared_ptr<> reference count and this could lead to a path where the\nobject held by the shared_ptr<> (the camera in this case) is released\nbefore the FrameBufferAllocator instance referencing it ?\n\nDo yuo think it's an issue ? Applications shouldn't\n\n        FrameBufferAllocator(std::move(camera))\n\nbut is there anything preventing them from doing so ?\n\n>  {\n>  }\n>\n> --\n> 2.44.0\n>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 1A3AABD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 11 Mar 2024 17:01:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7271E62868;\n\tMon, 11 Mar 2024 18:01:19 +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 4039661C7C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 11 Mar 2024 18:01:18 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EF1666BE;\n\tMon, 11 Mar 2024 18:00:56 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XIUywr4L\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1710176457;\n\tbh=M2rq1NUf920gbHDlPY4VHUXvbzCzhA7OoaPYTv7FLgE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XIUywr4LidvAy7d8n/2/5eEjnkqc12vT71tTls2hz1r0fGn/+MfRGS9Lgb6GFRbrf\n\tV+5GsgwMmVcnatoJz0fFpU/is1n1CX2uvi9pMcQTTRp33WeFgN+jkCxKZDujwWr/1d\n\tIQ2DGgAm+6fG70fLbovpOBMqdK66oL+fUmAp2zG4=","Date":"Mon, 11 Mar 2024 18:01:15 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Subject":"Re: [PATCH v2 1/3] libcamera: framebuffer_allocator: Move from\n\targument in constructor","Message-ID":"<w3k2jud53xfxktv3omg26ff3gi435lhur6ldtdqfkqe2dqjht4@dbqiahab4ciy>","References":"<20240310143023.752559-1-pobrn@protonmail.com>\n\t<20240310143023.752559-2-pobrn@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20240310143023.752559-2-pobrn@protonmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28923,"web_url":"https://patchwork.libcamera.org/comment/28923/","msgid":"<fKWwWwlbKpskqjGLwg5zHxabe6JLhmFh9zpb9CZUiAPnFrCn3efRnaYLm0CNyAuYs8z89CV4PyDK1RLt96JM54ZyTsbbx4sSDwexDlgO0IA=@protonmail.com>","date":"2024-03-11T17:42:55","subject":"Re: [PATCH v2 1/3] libcamera: framebuffer_allocator: Move from\n\targument in constructor","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. március 11., hétfő 18:01 keltezéssel, Jacopo Mondi <jacopo.mondi@ideasonboard.com> írta:\n\n> Hi Barnabás\n> \n> On Sun, Mar 10, 2024 at 02:30:33PM +0000, Barnabás Pőcze wrote:\n> > The single argument, of type `std::shared_ptr<Camera>`,\n> > is passed by value, so it can simply be moved from in order to\n> > avoid calling the copy constructor.\n> >\n> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> > ---\n> >  src/libcamera/framebuffer_allocator.cpp | 2 +-\n> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\n> > index 94389735..8cf45ab2 100644\n> > --- a/src/libcamera/framebuffer_allocator.cpp\n> > +++ b/src/libcamera/framebuffer_allocator.cpp\n> > @@ -59,7 +59,7 @@ LOG_DEFINE_CATEGORY(Allocator)\n> >   * \\param[in] camera The camera\n> >   */\n> >  FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr<Camera> camera)\n> > -\t: camera_(camera)\n> > +\t: camera_(std::move(camera))\n> \n> However, if the caller of the FrameBufferAllocator constructor uses\n> move() when calling the constructur, this will not increase the\n> shared_ptr<> reference count and this could lead to a path where the\n> object held by the shared_ptr<> (the camera in this case) is released\n> before the FrameBufferAllocator instance referencing it ?\n\nThat is not possible. The constructor's argument holds a reference to the camera\nobject preventing it from being destroyed.\n\n\n> \n> Do yuo think it's an issue ? Applications shouldn't\n> \n>         FrameBufferAllocator(std::move(camera))\n> \n> but is there anything preventing them from doing so ?\n\nThe observable behaviour for the caller does not really change after the proposed change.\nIf someone uses `FrameBufferAllocator(std::move(camera))`, then that will construct\na temporary with the shared_ptr's move constructor, so `camera` will point to nullptr\nafterwards, regardless of what is done inside the constructor. This is what happens\nnow and what will happen after this change.\n\nPreviously if someone did\n\n  FrameBufferAllocator { ... }\n\nthen there would be\n  * two copy constructor invocations, or\n  * one move constructor and one copy constructor invocation.\ndepending on the type of `...` above.\n\nThe first one when constructing the temporary for the argument,\nand the second one when constructing the member variable from the argument.\n\nWith the proposed change, there will be at most one copy constructor call\nwhen constructing a FrameBufferAllocator.\n\n\n> \n> >  {\n> >  }\n> >\n> > --\n> > 2.44.0\n> >\n> >\n\n\nRegards,\nBarnabás Pőcze","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D34EBBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 11 Mar 2024 17:43:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2688B62868;\n\tMon, 11 Mar 2024 18:43:05 +0100 (CET)","from mail-40134.protonmail.ch (mail-40134.protonmail.ch\n\t[185.70.40.134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8FAF361C7C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 11 Mar 2024 18:43:03 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"Vs74T9M6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1710178982; x=1710438182;\n\tbh=T1o5sU8rO9EFemnO8jQD+y0/2dzlDo/3nZ15DOpkwRA=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=Vs74T9M6MS1+xtaPLWv9Ly7lebCycSe/9JhOyqmGx3ssszRAnGtALsBdr7AfIN0cd\n\tx95TUKMwJyRF4gLdGrp9R9kuMzHESu6LDp+iqSZRkWN4bEtgxUi+Y9/xISx/RJKcpO\n\tRN5prxpbBWbdcCujMYYTRhZnyIj+Ni2mQUwLhC5zL1slcNL60iXNkWQIAx2AUjy+p2\n\toamFsBcZfWl5q+s7wFdngUt2SISI7ntgUf6BX5Z/U68wlUnzfJB2wpWQVnzvyglYa7\n\tSPbJvxYjCiUesV5JkYDox/uhXRMrtQ8jJAOjsRKb4jQzFiE+c1UKxUzrl4AoJzJfoy\n\tD9gqg9dF8t66g==","Date":"Mon, 11 Mar 2024 17:42:55 +0000","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Subject":"Re: [PATCH v2 1/3] libcamera: framebuffer_allocator: Move from\n\targument in constructor","Message-ID":"<fKWwWwlbKpskqjGLwg5zHxabe6JLhmFh9zpb9CZUiAPnFrCn3efRnaYLm0CNyAuYs8z89CV4PyDK1RLt96JM54ZyTsbbx4sSDwexDlgO0IA=@protonmail.com>","In-Reply-To":"<w3k2jud53xfxktv3omg26ff3gi435lhur6ldtdqfkqe2dqjht4@dbqiahab4ciy>","References":"<20240310143023.752559-1-pobrn@protonmail.com>\n\t<20240310143023.752559-2-pobrn@protonmail.com>\n\t<w3k2jud53xfxktv3omg26ff3gi435lhur6ldtdqfkqe2dqjht4@dbqiahab4ciy>","Feedback-ID":"20568564:user:proton","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28924,"web_url":"https://patchwork.libcamera.org/comment/28924/","msgid":"<crdzjjuxvyzdoh3fjdgzu3drj6y2vhvkele6sqa3scbrxogjub@irhit752zijf>","date":"2024-03-12T08:46:08","subject":"Re: [PATCH v2 1/3] libcamera: framebuffer_allocator: Move from\n\targument in constructor","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Mon, Mar 11, 2024 at 05:42:55PM +0000, Barnabás Pőcze wrote:\n> Hi\n>\n>\n> 2024. március 11., hétfő 18:01 keltezéssel, Jacopo Mondi <jacopo.mondi@ideasonboard.com> írta:\n>\n> > Hi Barnabás\n> >\n> > On Sun, Mar 10, 2024 at 02:30:33PM +0000, Barnabás Pőcze wrote:\n> > > The single argument, of type `std::shared_ptr<Camera>`,\n> > > is passed by value, so it can simply be moved from in order to\n> > > avoid calling the copy constructor.\n> > >\n> > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> > > ---\n> > >  src/libcamera/framebuffer_allocator.cpp | 2 +-\n> > >  1 file changed, 1 insertion(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\n> > > index 94389735..8cf45ab2 100644\n> > > --- a/src/libcamera/framebuffer_allocator.cpp\n> > > +++ b/src/libcamera/framebuffer_allocator.cpp\n> > > @@ -59,7 +59,7 @@ LOG_DEFINE_CATEGORY(Allocator)\n> > >   * \\param[in] camera The camera\n> > >   */\n> > >  FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr<Camera> camera)\n> > > -\t: camera_(camera)\n> > > +\t: camera_(std::move(camera))\n> >\n> > However, if the caller of the FrameBufferAllocator constructor uses\n> > move() when calling the constructur, this will not increase the\n> > shared_ptr<> reference count and this could lead to a path where the\n> > object held by the shared_ptr<> (the camera in this case) is released\n> > before the FrameBufferAllocator instance referencing it ?\n>\n> That is not possible. The constructor's argument holds a reference to the camera\n> object preventing it from being destroyed.\n>\n>\n> >\n> > Do yuo think it's an issue ? Applications shouldn't\n> >\n> >         FrameBufferAllocator(std::move(camera))\n> >\n> > but is there anything preventing them from doing so ?\n>\n> The observable behaviour for the caller does not really change after the proposed change.\n> If someone uses `FrameBufferAllocator(std::move(camera))`, then that will construct\n> a temporary with the shared_ptr's move constructor, so `camera` will point to nullptr\n> afterwards, regardless of what is done inside the constructor. This is what happens\n> now and what will happen after this change.\n\nI think the key point is also that std::move() of a shared_ptr<>\neffectively resets it, so it's not something anyone would do, which\nmakes my argument moot\n\n>\n> Previously if someone did\n>\n>   FrameBufferAllocator { ... }\n>\n> then there would be\n>   * two copy constructor invocations, or\n>   * one move constructor and one copy constructor invocation.\n> depending on the type of `...` above.\n>\n> The first one when constructing the temporary for the argument,\n> and the second one when constructing the member variable from the argument.\n>\n> With the proposed change, there will be at most one copy constructor call\n> when constructing a FrameBufferAllocator.\n>\n\nFor reference, this is the code generated by goldbot for the two use\ncases with the following code snippet:\n\n-------------------------------------------------------------------------------\n#include <memory>\n\nusing namespace std;\n\nclass klass\n{\npublic:\n    klass(shared_ptr<int> i)\n        : i_(i)\n        // : i_(move(i))\n    {\n    }\n\nprivate:\n    shared_ptr<int> i_;\n};\n\nint main()\n{\n    shared_ptr<int> i = std::make_shared<int>(5);\n\n    klass k(i);\n\n    return 0;\n}\n-------------------------------------------------------------------------------\n\n\n* move)\n\tmov     rbx, QWORD PTR [rbp-24]\n        mov     rax, QWORD PTR [rbp-32]\n        mov     rdi, rax\n        call    std::remove_reference<std::shared_ptr<int>&>::type&& std::move<std::shared_ptr<int>&>(std::shared_ptr<int>&)\n        mov     rsi, rax\n        mov     rdi, rbx\n        call    std::shared_ptr<int>::shared_ptr(std::shared_ptr<int>&&) [complete object constructor]\n\n\n* non move)\n        mov     rax, QWORD PTR [rbp-8]\n        mov     rdx, QWORD PTR [rbp-16]\n        mov     rsi, rdx\n        mov     rdi, rax\n        call    std::shared_ptr<int>::shared_ptr(std::shared_ptr<int> const&) [complete object constructor]\n\nI hoped for the compiler to be smart enough to avoid calling the copy\nconstructor for both the temporary object and the class member\ninitialization, but that doesn't seem the case, so\n\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nThanks\n  j\n\n>\n> >\n> > >  {\n> > >  }\n> > >\n> > > --\n> > > 2.44.0\n> > >\n> > >\n>\n>\n> Regards,\n> Barnabás Pőcze","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 71BCBBD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 12 Mar 2024 08:46:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BABC062C84;\n\tTue, 12 Mar 2024 09:46:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 196FC61C7E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 12 Mar 2024 09:46:12 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 320AD593;\n\tTue, 12 Mar 2024 09:45:50 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bsW7wMJt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1710233150;\n\tbh=bQEh8UjdQUixNIRfphVrzn0dYWgmBJWhwHEGgubkkAA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bsW7wMJt8eg7AOp31XYTNMtZ7vTHTchc1Cina2I3KQKbKtwBc08BfO01rcuU63T2E\n\tUfUXe/Gj7aERCihQkTriiGx4Ew3DtH1YnDJ5moMBFki9WG2AdRlbITEpqmjpmhtJE5\n\tttgYS4Hi/qCnIdPSb81FZZS+r0sRUN0UKf9jJt+Q=","Date":"Tue, 12 Mar 2024 09:46:08 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Subject":"Re: [PATCH v2 1/3] libcamera: framebuffer_allocator: Move from\n\targument in constructor","Message-ID":"<crdzjjuxvyzdoh3fjdgzu3drj6y2vhvkele6sqa3scbrxogjub@irhit752zijf>","References":"<20240310143023.752559-1-pobrn@protonmail.com>\n\t<20240310143023.752559-2-pobrn@protonmail.com>\n\t<w3k2jud53xfxktv3omg26ff3gi435lhur6ldtdqfkqe2dqjht4@dbqiahab4ciy>\n\t<fKWwWwlbKpskqjGLwg5zHxabe6JLhmFh9zpb9CZUiAPnFrCn3efRnaYLm0CNyAuYs8z89CV4PyDK1RLt96JM54ZyTsbbx4sSDwexDlgO0IA=@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<fKWwWwlbKpskqjGLwg5zHxabe6JLhmFh9zpb9CZUiAPnFrCn3efRnaYLm0CNyAuYs8z89CV4PyDK1RLt96JM54ZyTsbbx4sSDwexDlgO0IA=@protonmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]