[{"id":24648,"web_url":"https://patchwork.libcamera.org/comment/24648/","msgid":"<20220818142352.7clqif6umozhr4vz@uno.localdomain>","date":"2022-08-18T14:23:52","subject":"Re: [libcamera-devel] [PATCH v3 06/17] py: Use UniqueFD","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Tomi\n\nOn Fri, Jul 01, 2022 at 11:45:10AM +0300, Tomi Valkeinen wrote:\n> Use UniqueFD to automate the eventfd lifetime management.\n>\n\nHere you go :)\n\nCan you squash with the previous patch ?\n\n> Signed-off-by: Tomi Valkeinen \n> ---\n> src/py/libcamera/py_camera_manager.cpp | 16 ++++------------\n> src/py/libcamera/py_camera_manager.h | 4 ++--\n> 2 files changed, 6 insertions(+), 14 deletions(-)\n>\n> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp\n> index ad47271d..51a890c8 100644\n> --- a/src/py/libcamera/py_camera_manager.cpp\n> +++ b/src/py/libcamera/py_camera_manager.cpp\n> @@ -27,25 +27,17 @@ PyCameraManager::PyCameraManager()\n> \t\tthrow std::system_error(errno, std::generic_category(),\n> \t\t\t\t\t\"Failed to create eventfd\");\n>\n> -\teventFd_ = fd;\n> +\teventFd_ = UniqueFD(fd);\n>\n> \tint ret = cameraManager_->start();\n> -\tif (ret) {\n> -\t\tclose(fd);\n> -\t\teventFd_ = -1;\n> +\tif (ret)\n> \t\tthrow std::system_error(-ret, std::generic_category(),\n> \t\t\t\t\t\"Failed to start CameraManager\");\n> -\t}\n> }\n>\n> PyCameraManager::~PyCameraManager()\n> {\n> \tLOG(Python, Debug) << \"~PyCameraManager()\";\n> -\n> -\tif (eventFd_ != -1) {\n> -\t\tclose(eventFd_);\n> -\t\teventFd_ = -1;\n> -\t}\n> }\n>\n> py::list PyCameraManager::cameras()\n> @@ -93,7 +85,7 @@ void PyCameraManager::writeFd()\n> {\n> \tuint64_t v = 1;\n>\n> -\tsize_t s = write(eventFd_, &v, 8);\n> +\tsize_t s = write(eventFd_.get(), &v, 8);\n> \t/*\n> \t * We should never fail, and have no simple means to manage the error,\n> \t * so let's log a fatal error.\n> @@ -106,7 +98,7 @@ void PyCameraManager::readFd()\n> {\n> \tuint8_t buf[8];\n>\n> -\tif (read(eventFd_, buf, 8) != 8)\n> +\tif (read(eventFd_.get(), buf, 8) != 8)\n> \t\tthrow std::system_error(errno, std::generic_category());\n> }\n>\n> diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h\n> index 9c15f814..710163e8 100644\n> --- a/src/py/libcamera/py_camera_manager.h\n> +++ b/src/py/libcamera/py_camera_manager.h\n> @@ -24,7 +24,7 @@ public:\n>\n> \tstatic const std::string &version() { return CameraManager::version(); }\n>\n> -\tint eventFd() const { return eventFd_; }\n> +\tint eventFd() const { return eventFd_.get(); }\n>\n> \tstd::vector getReadyRequests();\n>\n> @@ -33,7 +33,7 @@ public:\n> private:\n> \tstd::unique_ptr cameraManager_;\n>\n> -\tint eventFd_ = -1;\n> +\tUniqueFD eventFd_;\n> \tstd::mutex completedRequestsMutex_;\n> \tstd::vector completedRequests_;\n>\n> --\n> 2.34.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 268DAC3272\n\tfor ;\n\tThu, 18 Aug 2022 14:23:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E17AE61FC0;\n\tThu, 18 Aug 2022 16:23:56 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E63161FA7\n\tfor ;\n\tThu, 18 Aug 2022 16:23:55 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 0F94240007;\n\tThu, 18 Aug 2022 14:23:53 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660832636;\n\tbh=E21yq1Rl5StvS9sRcp3nMBmcf2IkCkOn+rnsHJLIWmA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=KETSoiIEkJ4BuufHfNtm9TyShE7KbCKvg/V+GOkY1ZsvVH0eTFemyNvyUMv2+Sj+0\n\tcs526X7mnTMRZuvYB87IKoEviC86IGGgPgdz+wATymQ1i4yv70fpwRULY9Ymp6oEOe\n\tNEBu8osoXtWsRN6/J3J9l/pqgVSEnRk0ItTrg6pTTJE4ezcxWwUKYM70BI2x1G9ICT\n\tYhW5o4z9bisGWPcDjXU235q1JCPD+tKgehThfyIp5/UEOE15PHoa54cFmsfiR+qFvZ\n\tWX0D1tIMeq/d861LY2XXXSMkVWWfTWYXQFyAlohv5gqOhuIwWq//Se44CZtzD03cYg\n\tE6Cm3OBQrlmrA==","Date":"Thu, 18 Aug 2022 16:23:52 +0200","To":"Tomi Valkeinen ","Message-ID":"<20220818142352.7clqif6umozhr4vz@uno.localdomain>","References":"<20220701084521.31831-1-tomi.valkeinen@ideasonboard.com>\n\t<20220701084521.31831-7-tomi.valkeinen@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220701084521.31831-7-tomi.valkeinen@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 06/17] py: Use UniqueFD","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24656,"web_url":"https://patchwork.libcamera.org/comment/24656/","msgid":"","date":"2022-08-18T14:43:46","subject":"Re: [libcamera-devel] [PATCH v3 06/17] py: Use UniqueFD","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 18/08/2022 17:23, Jacopo Mondi wrote:\n> Hi Tomi\n> \n> On Fri, Jul 01, 2022 at 11:45:10AM +0300, Tomi Valkeinen wrote:\n>> Use UniqueFD to automate the eventfd lifetime management.\n>>\n> \n> Here you go :)\n> \n> Can you squash with the previous patch ?\n\nNo, I don't think it's a good idea to squash changes like this. The \npoint of the previous patch was to convert the existing code to \nPyCameraManager. I don't do any changes there that are not needed.\n\nSquashing this, EFD_CLOEXEC and the mutex change would hide these \nchanges inside the large refactoring done in the previous patch.\n\n Tomi","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 5C4EAC3272\n\tfor ;\n\tThu, 18 Aug 2022 14:43:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B62DD61FC0;\n\tThu, 18 Aug 2022 16:43:50 +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 DA69361FA7\n\tfor ;\n\tThu, 18 Aug 2022 16:43:49 +0200 (CEST)","from [192.168.1.111] (91-158-154-79.elisa-laajakaista.fi\n\t[91.158.154.79])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1196A8B;\n\tThu, 18 Aug 2022 16:43:49 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660833830;\n\tbh=K/Fq/7Gqx0JKvsXnbzAc729tZXC5ckGtFCll/7YDUnQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=g7zYsrRW/3lkLUt4fZxZlajZ3u7jTk78Om0M1BOvnt/j7Ue116B1X5DVvwjihtyRD\n\tGd+CbcQ7ItOLFS877Fc/34yKjU71Hc6KeVG3GtiBtT6lp74iZW070PIxvJlmmdy/b9\n\tqd4o7E1n4AWv4NgW0dlnff9pbIacl+RPMawR4O1aJiyBby1kkNAYvURs3FT1fexQ2z\n\tuLrARzSV7culHtso2AQICHePlhodA6qUPGB+R1/TjnYKmlvd9MDDwEIoIFkG+rPQ9w\n\tHbifNx3Lcsa1VYcQHKCSx5laoe82dwZESAY6MPIPezIWHU5fdxUTlK2hLk6cTKx5wA\n\trCX7rxHLrKfpw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660833829;\n\tbh=K/Fq/7Gqx0JKvsXnbzAc729tZXC5ckGtFCll/7YDUnQ=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=TNJyXmP8fYrBcQSUweJFueNcgpfuNTIwrOvar7eXrTnsD4nHbuRQgYt+MOgi22kFg\n\tpscQUd0mXO/6fPO9lbvIWZ4mY2kkkkQEdor7xf9Y/diOBkWkwC97wamYnbuHP04+Um\n\tFwVD2dtrYpEGkdyjzKd2g24WIVolu97oKvEVvfQg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"TNJyXmP8\"; dkim-atps=neutral","Message-ID":"","Date":"Thu, 18 Aug 2022 17:43:46 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.11.0","Content-Language":"en-US","To":"Jacopo Mondi ","References":"<20220701084521.31831-1-tomi.valkeinen@ideasonboard.com>\n\t<20220701084521.31831-7-tomi.valkeinen@ideasonboard.com>\n\t<20220818142352.7clqif6umozhr4vz@uno.localdomain>","In-Reply-To":"<20220818142352.7clqif6umozhr4vz@uno.localdomain>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 06/17] py: Use UniqueFD","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","From":"Tomi Valkeinen via libcamera-devel\n\t","Reply-To":"Tomi Valkeinen ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24658,"web_url":"https://patchwork.libcamera.org/comment/24658/","msgid":"<20220818145110.caewkm7b37acis6o@uno.localdomain>","date":"2022-08-18T14:51:10","subject":"Re: [libcamera-devel] [PATCH v3 06/17] py: Use UniqueFD","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Thu, Aug 18, 2022 at 05:43:46PM +0300, Tomi Valkeinen wrote:\n> On 18/08/2022 17:23, Jacopo Mondi wrote:\n> > Hi Tomi\n> >\n> > On Fri, Jul 01, 2022 at 11:45:10AM +0300, Tomi Valkeinen wrote:\n> > > Use UniqueFD to automate the eventfd lifetime management.\n> > >\n> >\n> > Here you go :)\n> >\n> > Can you squash with the previous patch ?\n>\n> No, I don't think it's a good idea to squash changes like this. The point of\n> the previous patch was to convert the existing code to PyCameraManager. I\n> don't do any changes there that are not needed.\n>\n> Squashing this, EFD_CLOEXEC and the mutex change would hide these changes\n> inside the large refactoring done in the previous patch.\n>\n\nYou'reintroducing a new class, and then patching it on top.\nI understand you have moved existing code inside that class, but it's\na new component.\n\nAnyway, converting to use UniqueFD + EFD_CLOEXEC (or even better the\none liner eventfd_ = UniqueFD(eventfd(...));) can be made one change.\n\nAnway, it's a minor thing, up to you\n\n> Tomi","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 063B2BE173\n\tfor ;\n\tThu, 18 Aug 2022 14:51:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6BDC61FBC;\n\tThu, 18 Aug 2022 16:51:16 +0200 (CEST)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::226])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7951E61FA7\n\tfor ;\n\tThu, 18 Aug 2022 16:51:15 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 98CADC0003;\n\tThu, 18 Aug 2022 14:51:12 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660834276;\n\tbh=Vt+bfE2hoo8w4VyD7GK5IXfsmNeDxeBqdxDXG7fbM/M=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=U/jIlWxH9mxqhGJUQIXVxwYHODIuwl/HzMPUwesMfEDbyrecFGf/OIeBGr8FiHTDJ\n\t2hx/joByrv+V0IOGIFQ2FL3npw7n9xaDE6YFuE9c92RUuFWb2vOUPvaZHEABE/Z4Ec\n\tcK9gujmKbNNubtjrb5UEkmw3kaP1jSJjEJ7p5S7qlvtiEqocFUl+CvKkW8647vb1z6\n\toWKNDnwza2qwPIyDK+UQkfT6BFOBbQ+gP5qJKuX+pdQgEEMqTe/dESb3Ex8s+bF+Pj\n\tTSzdRcLt/ezeZcNEzsR71tVTHDhstQ5BGSzzQh0+FK8zoi7MrDWnsNBtzsCT3jb0CK\n\tasQC6wwCSzqzw==","Date":"Thu, 18 Aug 2022 16:51:10 +0200","To":"Tomi Valkeinen ","Message-ID":"<20220818145110.caewkm7b37acis6o@uno.localdomain>","References":"<20220701084521.31831-1-tomi.valkeinen@ideasonboard.com>\n\t<20220701084521.31831-7-tomi.valkeinen@ideasonboard.com>\n\t<20220818142352.7clqif6umozhr4vz@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3 06/17] py: Use UniqueFD","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24661,"web_url":"https://patchwork.libcamera.org/comment/24661/","msgid":"<61aa29f0-cec0-6e35-8889-e2a1ee373b85@ideasonboard.com>","date":"2022-08-18T15:01:59","subject":"Re: [libcamera-devel] [PATCH v3 06/17] py: Use UniqueFD","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 18/08/2022 17:51, Jacopo Mondi wrote:\n> \n> On Thu, Aug 18, 2022 at 05:43:46PM +0300, Tomi Valkeinen wrote:\n>> On 18/08/2022 17:23, Jacopo Mondi wrote:\n>>> Hi Tomi\n>>>\n>>> On Fri, Jul 01, 2022 at 11:45:10AM +0300, Tomi Valkeinen wrote:\n>>>> Use UniqueFD to automate the eventfd lifetime management.\n>>>>\n>>>\n>>> Here you go :)\n>>>\n>>> Can you squash with the previous patch ?\n>>\n>> No, I don't think it's a good idea to squash changes like this. The point of\n>> the previous patch was to convert the existing code to PyCameraManager. I\n>> don't do any changes there that are not needed.\n>>\n>> Squashing this, EFD_CLOEXEC and the mutex change would hide these changes\n>> inside the large refactoring done in the previous patch.\n>>\n> \n> You'reintroducing a new class, and then patching it on top.\n\nWell, I see it as encapsulating the existing code to be inside a class, \ni.e. moving code. =)\n\nIt is true that I could first do the small changes, and only after those \nchanges do the refactoring. I have them in the current order due to the \ntime the things were implemented.\n\n> I understand you have moved existing code inside that class, but it's\n> a new component.\n\nYes, but isn't it much more difficult to review if, while moving the \ncode inside a class, I also change the code (when not needed)?\n\n> Anyway, converting to use UniqueFD + EFD_CLOEXEC (or even better the\n> one liner eventfd_ = UniqueFD(eventfd(...));) can be made one change.\n\nTrue, but... Why? One shouldn't squash unrelated commits together. If I \ndid what you suggest, then in the commit descs I would have to write \nsomething like \"This patch does two things:...\" or first describe the \nfirst change and then continue \"also, we change...\". When you write such \nthings to a commit desc, it's almost a sure sign that you should split \nthe patch =).\n\n Tomi","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 85771C3272\n\tfor ;\n\tThu, 18 Aug 2022 15:02:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2DAFB61FC0;\n\tThu, 18 Aug 2022 17:02:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 124BC61FA7\n\tfor ;\n\tThu, 18 Aug 2022 17:02:03 +0200 (CEST)","from [192.168.1.111] (91-158-154-79.elisa-laajakaista.fi\n\t[91.158.154.79])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 54C9F8B;\n\tThu, 18 Aug 2022 17:02:02 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660834924;\n\tbh=ao6Wk6smduL1Ew7Fv/FQBnxdi8wWYfuiyURtHOKLWM4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=RuEd1dKNZziwUsM+pBue4/2NCE9UUt9HCeiM1Kc9gvU8mi7AUYcnBglIZyJzCeQZg\n\t3dwJkTAMbumU3/cccI8XwWBU2+4sCETi1Yez++psoQQmLzFuAPSWvH2E+xWGIoYTWZ\n\tqt+tR5FnfJuGquYgiNIL0rX3z6XQmGbpdtKvD/CM3UDz84hwqVF4cyGKtNvFV6YDGP\n\to+HSyVM4dsXTDOr9pNxg5FbMqs2DiTMuw5JlAZryuicpmeYKWPHqSt+V/pZADcCD2Y\n\tBbzzH5PYnH5KNsPAEG8tQfZ7e/M/9J8F2AG0AR5e8RTLqEWaAcc3XYZdXOGMkCxDnU\n\tFPVxPnfI8hhVA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660834922;\n\tbh=ao6Wk6smduL1Ew7Fv/FQBnxdi8wWYfuiyURtHOKLWM4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=aS/S5Q9MvZU5smnhOWv4Eb+j7MlrW6IiyvgcPGJodjkMTunOsed1p3GIHbG4ZX/hc\n\twT8QqvfMci2E2wX1Y+2+p3Ko4U8WMaid2pQQCoE5m6qAL2Oe7BuR4pppeasn3/z+KW\n\tbDk2i5M6TVyTJDMIXgB+Gf4taTFATGiQ94dJ0MWg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"aS/S5Q9M\"; dkim-atps=neutral","Message-ID":"<61aa29f0-cec0-6e35-8889-e2a1ee373b85@ideasonboard.com>","Date":"Thu, 18 Aug 2022 18:01:59 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.11.0","Content-Language":"en-US","To":"Jacopo Mondi ","References":"<20220701084521.31831-1-tomi.valkeinen@ideasonboard.com>\n\t<20220701084521.31831-7-tomi.valkeinen@ideasonboard.com>\n\t<20220818142352.7clqif6umozhr4vz@uno.localdomain>\n\t\n\t<20220818145110.caewkm7b37acis6o@uno.localdomain>","In-Reply-To":"<20220818145110.caewkm7b37acis6o@uno.localdomain>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 06/17] py: Use UniqueFD","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","From":"Tomi Valkeinen via libcamera-devel\n\t","Reply-To":"Tomi Valkeinen ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24666,"web_url":"https://patchwork.libcamera.org/comment/24666/","msgid":"","date":"2022-08-18T15:13:25","subject":"Re: [libcamera-devel] [PATCH v3 06/17] py: Use UniqueFD","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Tomi,\n\nThank you for the patch.\n\nOn Fri, Jul 01, 2022 at 11:45:10AM +0300, Tomi Valkeinen wrote:\n> Use UniqueFD to automate the eventfd lifetime management.\n> \n> Signed-off-by: Tomi Valkeinen \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/py/libcamera/py_camera_manager.cpp | 16 ++++------------\n> src/py/libcamera/py_camera_manager.h | 4 ++--\n> 2 files changed, 6 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp\n> index ad47271d..51a890c8 100644\n> --- a/src/py/libcamera/py_camera_manager.cpp\n> +++ b/src/py/libcamera/py_camera_manager.cpp\n> @@ -27,25 +27,17 @@ PyCameraManager::PyCameraManager()\n> \t\tthrow std::system_error(errno, std::generic_category(),\n> \t\t\t\t\t\"Failed to create eventfd\");\n> \n> -\teventFd_ = fd;\n> +\teventFd_ = UniqueFD(fd);\n> \n> \tint ret = cameraManager_->start();\n> -\tif (ret) {\n> -\t\tclose(fd);\n> -\t\teventFd_ = -1;\n> +\tif (ret)\n> \t\tthrow std::system_error(-ret, std::generic_category(),\n> \t\t\t\t\t\"Failed to start CameraManager\");\n> -\t}\n> }\n> \n> PyCameraManager::~PyCameraManager()\n> {\n> \tLOG(Python, Debug) << \"~PyCameraManager()\";\n> -\n> -\tif (eventFd_ != -1) {\n> -\t\tclose(eventFd_);\n> -\t\teventFd_ = -1;\n> -\t}\n> }\n> \n> py::list PyCameraManager::cameras()\n> @@ -93,7 +85,7 @@ void PyCameraManager::writeFd()\n> {\n> \tuint64_t v = 1;\n> \n> -\tsize_t s = write(eventFd_, &v, 8);\n> +\tsize_t s = write(eventFd_.get(), &v, 8);\n> \t/*\n> \t * We should never fail, and have no simple means to manage the error,\n> \t * so let's log a fatal error.\n> @@ -106,7 +98,7 @@ void PyCameraManager::readFd()\n> {\n> \tuint8_t buf[8];\n> \n> -\tif (read(eventFd_, buf, 8) != 8)\n> +\tif (read(eventFd_.get(), buf, 8) != 8)\n> \t\tthrow std::system_error(errno, std::generic_category());\n> }\n> \n> diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h\n> index 9c15f814..710163e8 100644\n> --- a/src/py/libcamera/py_camera_manager.h\n> +++ b/src/py/libcamera/py_camera_manager.h\n> @@ -24,7 +24,7 @@ public:\n> \n> \tstatic const std::string &version() { return CameraManager::version(); }\n> \n> -\tint eventFd() const { return eventFd_; }\n> +\tint eventFd() const { return eventFd_.get(); }\n> \n> \tstd::vector getReadyRequests();\n> \n> @@ -33,7 +33,7 @@ public:\n> private:\n> \tstd::unique_ptr cameraManager_;\n> \n> -\tint eventFd_ = -1;\n> +\tUniqueFD eventFd_;\n> \tstd::mutex completedRequestsMutex_;\n> \tstd::vector completedRequests_;\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 AF6CFC3272\n\tfor ;\n\tThu, 18 Aug 2022 15:13:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0D41B61FC0;\n\tThu, 18 Aug 2022 17:13:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EF2C161FA7\n\tfor ;\n\tThu, 18 Aug 2022 17:13:28 +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 378F58B;\n\tThu, 18 Aug 2022 17:13:28 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660835610;\n\tbh=eBFVNu/2GQtQ//mGB6ptZB74FGeCy8ewEtEK4hfpoos=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=hpNZz+BPTAhikjdRiCRGPkiQACTL/GBKt4AeNYS+THtm+hYNXvSaaGIAKxzNjYEKz\n\txQW2XFaG5ixBei5d8oVV7FlNMapEFDlgvc48dyYZRiIDm1yulDqj3nGMlqIA/3V7YZ\n\tu/7jJ3gjAhqy06D7sD+Tpzbtu5irfcmlyZVoOxvdr4LQMi/g8Tqvb0RMYR1wVOhYAB\n\tV041fviFpuI5eHB6zq+Cq0rd30D7i0ql1Th6F/HDbY413pgvPvNuCQPv4uDxmxUAze\n\tKbHODzUEFaeKlKYZSXHEIYH56pbaI+3/R5/EvEKHqeetQpBLHwlhypinlR8GjzUSMS\n\tSnRnuglqVhItw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660835608;\n\tbh=eBFVNu/2GQtQ//mGB6ptZB74FGeCy8ewEtEK4hfpoos=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WVXbkgLIkuKLzm6O0lgDIWhi+1vyhO+gR8xiBYUOr6jwYbu+3iQ77uAc9ZVAfedHM\n\tkv37gfIZlrTaYY10olXe9FkF4yio9A+kRNUe5mMIFbbZHqnwwFqPvMa/Rso/86x4Bo\n\tAGWaqFizlsys9g0Pr7L1Dz4K353M6jH2Veg6LcPU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"WVXbkgLI\"; dkim-atps=neutral","Date":"Thu, 18 Aug 2022 18:13:25 +0300","To":"Tomi Valkeinen ","Message-ID":"","References":"<20220701084521.31831-1-tomi.valkeinen@ideasonboard.com>\n\t<20220701084521.31831-7-tomi.valkeinen@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220701084521.31831-7-tomi.valkeinen@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 06/17] py: Use UniqueFD","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]