[{"id":19172,"web_url":"https://patchwork.libcamera.org/comment/19172/","msgid":"","date":"2021-08-30T12:40:45","subject":"Re: [libcamera-devel] [PATCH v1 5/6] libcamera: Don't use emitter\n\tobject pointer argument to slot","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 8/27/21 8:08 AM, Laurent Pinchart wrote:\n> In many cases, the emitter object passed as a pointer from signals to\n> slots is also available as a class member. Use the class member when\n> this occurs, to prepare for removal of the emitter object pointer from\n> signals.\n>\n> In test/event.cpp, this additionally requires moving the EventNotifier\n> to a class member.\n>\n> Signed-off-by: Laurent Pinchart \n\n\nReviewed-by: Umang Jain \n\n> ---\n> src/libcamera/ipc_pipe_unixsocket.cpp | 4 ++--\n> test/event-thread.cpp | 4 ++--\n> test/event.cpp | 17 +++++++++++------\n> test/ipa/ipa_interface_test.cpp | 4 ++--\n> test/ipc/unixsocket.cpp | 8 ++++----\n> test/ipc/unixsocket_ipc.cpp | 4 ++--\n> .../module_ipa_proxy_worker.cpp.tmpl | 4 ++--\n> 7 files changed, 25 insertions(+), 20 deletions(-)\n>\n> diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp\n> index 4511775fb467..38bcc30a21ed 100644\n> --- a/src/libcamera/ipc_pipe_unixsocket.cpp\n> +++ b/src/libcamera/ipc_pipe_unixsocket.cpp\n> @@ -82,10 +82,10 @@ int IPCPipeUnixSocket::sendAsync(const IPCMessage &data)\n> \treturn 0;\n> }\n> \n> -void IPCPipeUnixSocket::readyRead(IPCUnixSocket *socket)\n> +void IPCPipeUnixSocket::readyRead([[maybe_unused]] IPCUnixSocket *socket)\n> {\n> \tIPCUnixSocket::Payload payload;\n> -\tint ret = socket->receive(&payload);\n> +\tint ret = socket_->receive(&payload);\n> \tif (ret) {\n> \t\tLOG(IPCPipe, Error) << \"Receive message failed\" << ret;\n> \t\treturn;\n> diff --git a/test/event-thread.cpp b/test/event-thread.cpp\n> index 575261664c2f..12021710ef41 100644\n> --- a/test/event-thread.cpp\n> +++ b/test/event-thread.cpp\n> @@ -66,9 +66,9 @@ public:\n> \t}\n> \n> private:\n> -\tvoid readReady(EventNotifier *notifier)\n> +\tvoid readReady([[maybe_unused]] EventNotifier *notifier)\n> \t{\n> -\t\tsize_ = read(notifier->fd(), data_, sizeof(data_));\n> +\t\tsize_ = read(notifier_->fd(), data_, sizeof(data_));\n> \t\tnotified_ = true;\n> \t}\n> \n> diff --git a/test/event.cpp b/test/event.cpp\n> index c2274344b7f0..e338335c11e8 100644\n> --- a/test/event.cpp\n> +++ b/test/event.cpp\n> @@ -22,14 +22,16 @@ using namespace libcamera;\n> class EventTest : public Test\n> {\n> protected:\n> -\tvoid readReady(EventNotifier *notifier)\n> +\tvoid readReady([[maybe_unused]] EventNotifier *notifier)\n> \t{\n> -\t\tsize_ = read(notifier->fd(), data_, sizeof(data_));\n> +\t\tsize_ = read(notifier_->fd(), data_, sizeof(data_));\n> \t\tnotified_ = true;\n> \t}\n> \n> \tint init()\n> \t{\n> +\t\tnotifier_ = nullptr;\n> +\n> \t\treturn pipe(pipefd_);\n> \t}\n> \n> @@ -40,8 +42,8 @@ protected:\n> \t\tTimer timeout;\n> \t\tssize_t ret;\n> \n> -\t\tEventNotifier readNotifier(pipefd_[0], EventNotifier::Read);\n> -\t\treadNotifier.activated.connect(this, &EventTest::readReady);\n> +\t\tnotifier_ = new EventNotifier(pipefd_[0], EventNotifier::Read);\n> +\t\tnotifier_->activated.connect(this, &EventTest::readReady);\n> \n> \t\t/* Test read notification with data. */\n> \t\tmemset(data_, 0, sizeof(data_));\n> @@ -76,7 +78,7 @@ protected:\n> \n> \t\t/* Test read notifier disabling. */\n> \t\tnotified_ = false;\n> -\t\treadNotifier.setEnabled(false);\n> +\t\tnotifier_->setEnabled(false);\n> \n> \t\tret = write(pipefd_[1], data.data(), data.size());\n> \t\tif (ret < 0) {\n> @@ -95,7 +97,7 @@ protected:\n> \n> \t\t/* Test read notifier enabling. */\n> \t\tnotified_ = false;\n> -\t\treadNotifier.setEnabled(true);\n> +\t\tnotifier_->setEnabled(true);\n> \n> \t\ttimeout.start(100);\n> \t\tdispatcher->processEvents();\n> @@ -111,6 +113,8 @@ protected:\n> \n> \tvoid cleanup()\n> \t{\n> +\t\tdelete notifier_;\n> +\n> \t\tclose(pipefd_[0]);\n> \t\tclose(pipefd_[1]);\n> \t}\n> @@ -118,6 +122,7 @@ protected:\n> private:\n> \tint pipefd_[2];\n> \n> +\tEventNotifier *notifier_;\n> \tbool notified_;\n> \tchar data_[16];\n> \tssize_t size_;\n> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n> index ee9f26510784..0ee51f71fd87 100644\n> --- a/test/ipa/ipa_interface_test.cpp\n> +++ b/test/ipa/ipa_interface_test.cpp\n> @@ -153,9 +153,9 @@ protected:\n> \t}\n> \n> private:\n> -\tvoid readTrace(EventNotifier *notifier)\n> +\tvoid readTrace([[maybe_unused]] EventNotifier *notifier)\n> \t{\n> -\t\tssize_t s = read(notifier->fd(), &trace_, sizeof(trace_));\n> +\t\tssize_t s = read(notifier_->fd(), &trace_, sizeof(trace_));\n> \t\tif (s < 0) {\n> \t\t\tint ret = errno;\n> \t\t\tcerr << \"Failed to read from IPA test FIFO at '\"\n> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> index aa35c8f071f1..6507fb12d74b 100644\n> --- a/test/ipc/unixsocket.cpp\n> +++ b/test/ipc/unixsocket.cpp\n> @@ -68,12 +68,12 @@ public:\n> \t}\n> \n> private:\n> -\tvoid readyRead(IPCUnixSocket *ipc)\n> +\tvoid readyRead([[maybe_unused]] IPCUnixSocket *ipc)\n> \t{\n> \t\tIPCUnixSocket::Payload message, response;\n> \t\tint ret;\n> \n> -\t\tret = ipc->receive(&message);\n> +\t\tret = ipc_.receive(&message);\n> \t\tif (ret) {\n> \t\t\tcerr << \"Receive message failed: \" << ret << endl;\n> \t\t\treturn;\n> @@ -447,14 +447,14 @@ private:\n> \t\treturn 0;\n> \t}\n> \n> -\tvoid readyRead(IPCUnixSocket *ipc)\n> +\tvoid readyRead([[maybe_unused]] IPCUnixSocket *ipc)\n> \t{\n> \t\tif (!callResponse_) {\n> \t\t\tcerr << \"Read ready without expecting data, fail.\" << endl;\n> \t\t\treturn;\n> \t\t}\n> \n> -\t\tif (ipc->receive(callResponse_)) {\n> +\t\tif (ipc_.receive(callResponse_)) {\n> \t\t\tcerr << \"Receive message failed\" << endl;\n> \t\t\treturn;\n> \t\t}\n> diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp\n> index 6fe7fd9b8fc5..60317a4956b8 100644\n> --- a/test/ipc/unixsocket_ipc.cpp\n> +++ b/test/ipc/unixsocket_ipc.cpp\n> @@ -65,12 +65,12 @@ public:\n> \t}\n> \n> private:\n> -\tvoid readyRead(IPCUnixSocket *ipc)\n> +\tvoid readyRead([[maybe_unused]] IPCUnixSocket *ipc)\n> \t{\n> \t\tIPCUnixSocket::Payload message;\n> \t\tint ret;\n> \n> -\t\tret = ipc->receive(&message);\n> +\t\tret = ipc_.receive(&message);\n> \t\tif (ret) {\n> \t\t\tcerr << \"Receive message failed: \" << ret << endl;\n> \t\t\treturn;\n> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n> index 8a88bd467da7..b4cd1aa9e823 100644\n> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n> @@ -57,10 +57,10 @@ public:\n> \n> \t~{{proxy_worker_name}}() {}\n> \n> -\tvoid readyRead(IPCUnixSocket *socket)\n> +\tvoid readyRead([[maybe_unused]] IPCUnixSocket *socket)\n> \t{\n> \t\tIPCUnixSocket::Payload _message;\n> -\t\tint _retRecv = socket->receive(&_message);\n> +\t\tint _retRecv = socket_.receive(&_message);\n> \t\tif (_retRecv) {\n> \t\t\tLOG({{proxy_worker_name}}, Error)\n> \t\t\t\t<< \"Receive message failed: \" << _retRecv;","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 457BFBD87D\n\tfor ;\n\tMon, 30 Aug 2021 12:40:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 884846916C;\n\tMon, 30 Aug 2021 14:40:51 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 33FC960258\n\tfor ;\n\tMon, 30 Aug 2021 14:40:50 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.0])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 401815A7;\n\tMon, 30 Aug 2021 14:40:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"QQBHFHqO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630327249;\n\tbh=gRVWD+egylgfjVDhrufaep2RBvmhrSx3dY8GTfnmrms=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=QQBHFHqOGsMXPD7C4P8AZYLrS4Cq9RbBXjdR3oOs22suCOJX8GmEZsUKTEN7jAoNR\n\tZ0E6Mf7ozVYf17ctfkwD+IRpbdXo4h4gaONwAblYd/QptpozqbyDML+QvdddAWbAkf\n\tl0Nk9ztDihQBGkCXdM+zllqNbyGTKC1unTnA9qJU=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210827023829.5871-1-laurent.pinchart@ideasonboard.com>\n\t<20210827023829.5871-6-laurent.pinchart@ideasonboard.com>","From":"Umang Jain ","Message-ID":"","Date":"Mon, 30 Aug 2021 18:10:45 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210827023829.5871-6-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v1 5/6] libcamera: Don't use emitter\n\tobject pointer argument to slot","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]