[{"id":16312,"web_url":"https://patchwork.libcamera.org/comment/16312/","msgid":"","date":"2021-04-16T13:15:25","subject":"Re: [libcamera-devel] [PATCH v2 2/3] ipa: raspberrypi: Add move\n\toperator to RPiController::Metadata","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for adding this function.\n\nOn Fri, 16 Apr 2021 at 11:31, Naushir Patuck wrote:\n>\n> Add the move operator implementation for the RPiController::Metadata\n> class.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/controller/metadata.hpp | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n>\n> diff --git a/src/ipa/raspberrypi/controller/metadata.hpp b/src/ipa/raspberrypi/controller/metadata.hpp\n> index 4f44ffc6771c..c55194040f2f 100644\n> --- a/src/ipa/raspberrypi/controller/metadata.hpp\n> +++ b/src/ipa/raspberrypi/controller/metadata.hpp\n> @@ -45,6 +45,14 @@ public:\n> data_ = other.data_;\n> return *this;\n> }\n> + Metadata &operator=(Metadata &&other)\n> + {\n> + std::lock_guard lock(mutex_);\n> + std::lock_guard other_lock(other.mutex_);\n> + data_ = std::move(other.data_);\n> + other.data_.clear();\n\nI did wonder slightly whether you need the explicit \"clear\" here, but\nI guess you probably do - it's left in a \"valid but unspecified\" state\n(even though in practice that would normally turn out to be empty).\nSo:\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> + return *this;\n> + }\n> template T *GetLocked(std::string const &tag)\n> {\n> // This allows in-place access to the Metadata contents,\n> --\n> 2.25.1\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 31731BD235\n\tfor ;\n\tFri, 16 Apr 2021 13:15:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9C9BF68812;\n\tFri, 16 Apr 2021 15:15:39 +0200 (CEST)","from mail-ot1-x32a.google.com (mail-ot1-x32a.google.com\n\t[IPv6:2607:f8b0:4864:20::32a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DCEE668806\n\tfor ;\n\tFri, 16 Apr 2021 15:15:37 +0200 (CEST)","by mail-ot1-x32a.google.com with SMTP id\n\td3-20020a9d29030000b029027e8019067fso23771217otb.13\n\tfor ;\n\tFri, 16 Apr 2021 06:15:37 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"KuSY2oTR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=pMguc+2Ml1X8t5RdIKD74ghQoR5HHez9fiCgLyOBfBo=;\n\tb=KuSY2oTRqZ7V5UYl0/2ppv1mbBaxCPYX4wPrZ63x3K2WLYQ8KGSRBoZ6844K/rMn2a\n\tcM3QIc6A07K2HthocoLuvZu59OOo2+ewngMPyY2jU2h0sXWh1H5jtC7UiPMdafc3xDGH\n\tJvALAYKqTph+Rotdr3OAIYMJAISNVs04cv9DeORD5XaK19wZVFDKqVZUhpCkMkDm7byj\n\tyhfZTkmPxW0166vJUsjcpULd3JME5uz8bI9o/4iL2bnX/C3n7U/MUHPJm4PWjORyLiKi\n\ta9IPtdEYXZW+ZJE56AQJ1QDSv3r+M1fJ+bCa3XCRQgczobkX2xJfscRk9qTPsZbSaMpE\n\tcsjw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=pMguc+2Ml1X8t5RdIKD74ghQoR5HHez9fiCgLyOBfBo=;\n\tb=cKYkbrxUfBN9xm8WIFn1zkGNlSDv8UMc4iVjkSb6cqWVA/5pZqh78zC5hAsTo63wHh\n\t7w/fXUI+Gj8Mnp0e/iXzTTGfphpjKhM6uihcv4J0Q6x5yNJ01aejINwl8TrACD59WjqA\n\ta6vQKhTANuecRvwB4puyBetQovAVx2I8HYUfeob+5gPNjh7r2SovCqlXZ+sbktDyd1IC\n\tGFczj/kKBKW6PY31vfZvkbH4MxYS7SdkIMvUaDcB3jR6MJp0EQ9CWXGZEU9ssSbozoHa\n\tPPOcOmP5nFFFj0SAYQQWNbYc2SPFfU7AS3oeu2ivogxZDqu50mTjkWt9nT3XwR1p4AQR\n\tt58g==","X-Gm-Message-State":"AOAM530Nymxc6jjhKxmn2K8TTeM95W20VasSzSa26/rZnWf5uFP6p/eN\n\twkw1YbRPbhMj3XOQdXD6Ta34xtKapEE8jytKhlPLKg==","X-Google-Smtp-Source":"ABdhPJzIuvk2/fH61wIgvKiMLg1NI/PH1GCiKe1L+I3g3N9Cr5motEha9r5r9nD7lyL9uvb7XyC9pJO4utHNZJXbAYc=","X-Received":"by 2002:a05:6830:c5:: with SMTP id\n\tx5mr3423668oto.317.1618578936880; \n\tFri, 16 Apr 2021 06:15:36 -0700 (PDT)","MIME-Version":"1.0","References":"<20210416103141.1483745-1-naush@raspberrypi.com>\n\t<20210416103141.1483745-3-naush@raspberrypi.com>","In-Reply-To":"<20210416103141.1483745-3-naush@raspberrypi.com>","From":"David Plowman ","Date":"Fri, 16 Apr 2021 14:15:25 +0100","Message-ID":"","To":"Naushir Patuck ","Subject":"Re: [libcamera-devel] [PATCH v2 2/3] ipa: raspberrypi: Add move\n\toperator to RPiController::Metadata","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":16321,"web_url":"https://patchwork.libcamera.org/comment/16321/","msgid":"","date":"2021-04-16T16:43:18","subject":"Re: [libcamera-devel] [PATCH v2 2/3] ipa: raspberrypi: Add move\n\toperator to RPiController::Metadata","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Fri, Apr 16, 2021 at 11:31:40AM +0100, Naushir Patuck wrote:\n> Add the move operator implementation for the RPiController::Metadata\n> class.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/controller/metadata.hpp | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n> \n> diff --git a/src/ipa/raspberrypi/controller/metadata.hpp b/src/ipa/raspberrypi/controller/metadata.hpp\n> index 4f44ffc6771c..c55194040f2f 100644\n> --- a/src/ipa/raspberrypi/controller/metadata.hpp\n> +++ b/src/ipa/raspberrypi/controller/metadata.hpp\n> @@ -45,6 +45,14 @@ public:\n> \t\tdata_ = other.data_;\n> \t\treturn *this;\n> \t}\n> +\tMetadata &operator=(Metadata &&other)\n> +\t{\n> +\t\tstd::lock_guard lock(mutex_);\n> +\t\tstd::lock_guard other_lock(other.mutex_);\n> +\t\tdata_ = std::move(other.data_);\n> +\t\tother.data_.clear();\n> +\t\treturn *this;\n> +\t}\n\nhttps://en.cppreference.com/w/cpp/language/rule_of_three says you should\nalso implement the copy and move constructors.\n\nI'm a bit concerned by the locks, but that's a separate question.\n\n> \ttemplate T *GetLocked(std::string const &tag)\n> \t{\n> \t\t// This allows in-place access to the Metadata contents,","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 77D97BD235\n\tfor ;\n\tFri, 16 Apr 2021 16:43:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AAF016880A;\n\tFri, 16 Apr 2021 18:43:22 +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 2513768806\n\tfor ;\n\tFri, 16 Apr 2021 18:43:21 +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 A03255A5;\n\tFri, 16 Apr 2021 18:43:20 +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=\"evmLNCDR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1618591400;\n\tbh=zYUSFyOSw2g/GhEZDfqBxR8T6Z4niAAsbrF3txvcMx8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=evmLNCDRGVPUfuKSZPTuG8kQ5VAhLurLR3wt5RYjhbE/tV0UFoOXBOqfJpTx651Qn\n\tbY5DQxTV6ircAeT6CW2gtwgfO6M20gRdPWeAciN8cA6TzMxyLe9LV2V2Y9icX5KlGw\n\tPGs2qUVlOw4FDVfkY4mrI6klGw7sdwKKVRHlI4JU=","Date":"Fri, 16 Apr 2021 19:43:18 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210416103141.1483745-1-naush@raspberrypi.com>\n\t<20210416103141.1483745-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210416103141.1483745-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/3] ipa: raspberrypi: Add move\n\toperator to RPiController::Metadata","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@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16334,"web_url":"https://patchwork.libcamera.org/comment/16334/","msgid":"","date":"2021-04-18T08:58:50","subject":"Re: [libcamera-devel] [PATCH v2 2/3] ipa: raspberrypi: Add move\n\toperator to RPiController::Metadata","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your feedback.\n\nOn Fri, 16 Apr 2021 at 17:43, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Fri, Apr 16, 2021 at 11:31:40AM +0100, Naushir Patuck wrote:\n> > Add the move operator implementation for the RPiController::Metadata\n> > class.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/ipa/raspberrypi/controller/metadata.hpp | 8 ++++++++\n> > 1 file changed, 8 insertions(+)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/metadata.hpp\n> b/src/ipa/raspberrypi/controller/metadata.hpp\n> > index 4f44ffc6771c..c55194040f2f 100644\n> > --- a/src/ipa/raspberrypi/controller/metadata.hpp\n> > +++ b/src/ipa/raspberrypi/controller/metadata.hpp\n> > @@ -45,6 +45,14 @@ public:\n> > data_ = other.data_;\n> > return *this;\n> > }\n> > + Metadata &operator=(Metadata &&other)\n> > + {\n> > + std::lock_guard lock(mutex_);\n> > + std::lock_guard other_lock(other.mutex_);\n> > + data_ = std::move(other.data_);\n> > + other.data_.clear();\n> > + return *this;\n> > + }\n>\n> https://en.cppreference.com/w/cpp/language/rule_of_three says you should\n> also implement the copy and move constructors.\n>\n\nIndeed! I'll add the copy and move constructors in the next revision.\n\n\n>\n> I'm a bit concerned by the locks, but that's a separate question.\n>\n\nFire away :-)\n\nRegards,\nNaush\n\n\n>\n> > template T *GetLocked(std::string const &tag)\n> > {\n> > // This allows in-place access to the Metadata contents,\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 22FC9BD810\n\tfor ;\n\tSun, 18 Apr 2021 08:59:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1EDC168806;\n\tSun, 18 Apr 2021 10:59:09 +0200 (CEST)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D4EE9602CD\n\tfor ;\n\tSun, 18 Apr 2021 10:59:06 +0200 (CEST)","by mail-lf1-x136.google.com with SMTP id f17so44291691lfu.7\n\tfor ;\n\tSun, 18 Apr 2021 01:59:06 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"qwIVfvG5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=+wDLtIbo2ndM7a9jy1KtCIOC1DWNXEKPCXdUwsHwNmc=;\n\tb=qwIVfvG5dawVVDS+Y9GIbU9rLL9WXFjPiywIfvrRflr2IT6vYV6bvZFzXQuqQ4qNh0\n\tfd+aiBC2hoUDxi4ZCvcREwWX3884FTqef+gJL6pgTevn7bhlX4SiIiCk5xl3YJYWAn/i\n\tE8d9irzIRWbtU7n5iA/QdjYvAT3zsXi1TAIt3l7KbK6P3w8/WBuMv0rz1JobxASWBufk\n\tqOtwauLsb//hF8Hlbke0tM1BdXnIlmupG7bvzC2hVoaVE5ObwECiyYdQMASXwsmo03N6\n\tY+bN44QjdZgPB6H/Q9uW0uaR6eMqvfCYoi60qJL/99dBTTjni2QbMtFYEtKND27dRAKF\n\tVXLg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=+wDLtIbo2ndM7a9jy1KtCIOC1DWNXEKPCXdUwsHwNmc=;\n\tb=uZwkiNwNOg4Qam3kleq/F1/DuYfFWw6S1n0eVON/r49e3A6UqYEvuXMxl7ptseV5zt\n\t90rUa7gJFq1msKLnG+IavmgrWDJcuhqPERk1JF245Zut7fv+B4l6sPy1IFrACV8ThNJx\n\t1R90OFEJ2iXHmqUPoj4iP7UjjG+LFxAyUuGXntXwc7vSAXmhQ4yDHBe+dq2gWWYSERK4\n\t4gzlZ+V2Si8VoA/cUu7FB81gUX1Vx1WSAOuWryxBX2R6UNFoTwaa2V5OYio72vEWIwpv\n\tWTeljz/OU907i2mUkAW3HRb+MwrmXN6grKo36HFB7sOa2bACCj742K/f7QwZ71nFFFaM\n\tlO3Q==","X-Gm-Message-State":"AOAM533pp33/YuSXobYlZU3WzYFte2ZXJ/ZivoqaJfTSYZiIxZkZnW43\n\t2KVx1Jm3bdNN8Xop5/aqhXHCc0AiMFITFATUbJRL3A==","X-Google-Smtp-Source":"ABdhPJzeWqTELRgYo7uyqHMl/uil5tBd/AIShGcayvtZLsLkqg92DB9mX98Dh1Vjh9OWUt1lugqeaeSTtVzqSPvKt0M=","X-Received":"by 2002:a05:6512:922:: with SMTP id\n\tf2mr8130906lft.171.1618736346156; \n\tSun, 18 Apr 2021 01:59:06 -0700 (PDT)","MIME-Version":"1.0","References":"<20210416103141.1483745-1-naush@raspberrypi.com>\n\t<20210416103141.1483745-3-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Sun, 18 Apr 2021 09:58:50 +0100","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v2 2/3] ipa: raspberrypi: Add move\n\toperator to RPiController::Metadata","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":"multipart/mixed;\n\tboundary=\"===============5263193676604114948==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]