Patch Detail
Show a patch.
GET /api/patches/14825/?format=api
{ "id": 14825, "url": "https://patchwork.libcamera.org/api/patches/14825/?format=api", "web_url": "https://patchwork.libcamera.org/patch/14825/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20211128235752.10836-11-laurent.pinchart@ideasonboard.com>", "date": "2021-11-28T23:57:45", "name": "[libcamera-devel,v3,10/17] libcamera: ipc_unixsocket: Use UniqueFD for a file descriptor", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "79af7c4b48936eb7ce530be08eac952f65666fc2", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/14825/mbox/", "series": [ { "id": 2768, "url": "https://patchwork.libcamera.org/api/series/2768/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2768", "date": "2021-11-28T23:57:35", "name": "libcamera: Introduce UniqueFD", "version": 3, "mbox": "https://patchwork.libcamera.org/series/2768/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/14825/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/14825/checks/", "tags": {}, "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 C5D2BC324F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 28 Nov 2021 23:58:34 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6CDD0605B8;\n\tMon, 29 Nov 2021 00:58:34 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B8D786059C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Nov 2021 00:58:25 +0100 (CET)", "from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 49EF4A15;\n\tMon, 29 Nov 2021 00:58:25 +0100 (CET)" ], "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=\"GFP9horl\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638143905;\n\tbh=FcuYDBN8znoPPe2i1sr31rqVNT8kLvY4+JAAGcrq4YY=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=GFP9horlufSrGjLQFZnUr5d10BtcP2qzLlkzxNYTdSVIRUcwsTgmEBawIISS0Uw4u\n\tlhsD53IDeUiIrnVEFgRCEATSKlIrspQo8ifsS/D8JyDZYU1s2ov9lD9SiFy25tKP54\n\tjAv1vUkaZGkTGecySl62RxAZmzmzHntZZft2VrBY=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 29 Nov 2021 01:57:45 +0200", "Message-Id": "<20211128235752.10836-11-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.32.0", "In-Reply-To": "<20211128235752.10836-1-laurent.pinchart@ideasonboard.com>", "References": "<20211128235752.10836-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v3 10/17] libcamera: ipc_unixsocket: Use\n\tUniqueFD for a file descriptor", "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>" }, "content": "From: Hirokazu Honda <hiroh@chromium.org>\n\nIPCUnixSocket::create() creates two file descriptors. One of\nthem is stored in IPCUnixSocket and the other is returned to a\ncaller. This clarifies the ownership using UniqueFD.\n\nSigned-off-by: Hirokazu Honda <hiroh@chromium.org>\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\nChanges since v2:\n\n- Pass UniqueFD to IPCUnixSocket::bind()\n- Return {}\n---\n include/libcamera/internal/ipc_unixsocket.h | 7 +--\n src/libcamera/ipc_pipe_unixsocket.cpp | 8 ++--\n src/libcamera/ipc_unixsocket.cpp | 43 ++++++++++---------\n test/ipc/unixsocket.cpp | 14 +++---\n test/ipc/unixsocket_ipc.cpp | 8 ++--\n .../module_ipa_proxy_worker.cpp.tmpl | 13 +++---\n 6 files changed, 49 insertions(+), 44 deletions(-)", "diff": "diff --git a/include/libcamera/internal/ipc_unixsocket.h b/include/libcamera/internal/ipc_unixsocket.h\nindex 5010b66a2bda..3963d182ffa6 100644\n--- a/include/libcamera/internal/ipc_unixsocket.h\n+++ b/include/libcamera/internal/ipc_unixsocket.h\n@@ -12,6 +12,7 @@\n #include <vector>\n \n #include <libcamera/base/signal.h>\n+#include <libcamera/base/unique_fd.h>\n \n namespace libcamera {\n \n@@ -28,8 +29,8 @@ public:\n \tIPCUnixSocket();\n \t~IPCUnixSocket();\n \n-\tint create();\n-\tint bind(int fd);\n+\tUniqueFD create();\n+\tint bind(UniqueFD fd);\n \tvoid close();\n \tbool isBound() const;\n \n@@ -49,7 +50,7 @@ private:\n \n \tvoid dataNotifier();\n \n-\tint fd_;\n+\tUniqueFD fd_;\n \tbool headerReceived_;\n \tstruct Header header_;\n \tEventNotifier *notifier_;\ndiff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp\nindex 533560cf95d3..65277500ff42 100644\n--- a/src/libcamera/ipc_pipe_unixsocket.cpp\n+++ b/src/libcamera/ipc_pipe_unixsocket.cpp\n@@ -31,14 +31,14 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,\n \targs.push_back(ipaModulePath);\n \n \tsocket_ = std::make_unique<IPCUnixSocket>();\n-\tint fd = socket_->create();\n-\tif (fd < 0) {\n+\tUniqueFD fd = socket_->create();\n+\tif (!fd.isValid()) {\n \t\tLOG(IPCPipe, Error) << \"Failed to create socket\";\n \t\treturn;\n \t}\n \tsocket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead);\n-\targs.push_back(std::to_string(fd));\n-\tfds.push_back(fd);\n+\targs.push_back(std::to_string(fd.get()));\n+\tfds.push_back(fd.release());\n \n \tproc_ = std::make_unique<Process>();\n \tint ret = proc_->start(ipaProxyWorkerPath, args, fds);\ndiff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\nindex bd32fca3a678..1980d374cea8 100644\n--- a/src/libcamera/ipc_unixsocket.cpp\n+++ b/src/libcamera/ipc_unixsocket.cpp\n@@ -7,6 +7,7 @@\n \n #include \"libcamera/internal/ipc_unixsocket.h\"\n \n+#include <array>\n #include <poll.h>\n #include <string.h>\n #include <sys/socket.h>\n@@ -68,7 +69,7 @@ LOG_DEFINE_CATEGORY(IPCUnixSocket)\n */\n \n IPCUnixSocket::IPCUnixSocket()\n-\t: fd_(-1), headerReceived_(false), notifier_(nullptr)\n+\t: headerReceived_(false), notifier_(nullptr)\n {\n }\n \n@@ -86,9 +87,9 @@ IPCUnixSocket::~IPCUnixSocket()\n * to the remote process, where it can be used with IPCUnixSocket::bind() to\n * bind the remote side socket.\n *\n- * \\return A file descriptor on success, negative error code on failure\n+ * \\return A file descriptor. It is valid on success or invalid otherwise.\n */\n-int IPCUnixSocket::create()\n+UniqueFD IPCUnixSocket::create()\n {\n \tint sockets[2];\n \tint ret;\n@@ -98,14 +99,18 @@ int IPCUnixSocket::create()\n \t\tret = -errno;\n \t\tLOG(IPCUnixSocket, Error)\n \t\t\t<< \"Failed to create socket pair: \" << strerror(-ret);\n-\t\treturn ret;\n+\t\treturn {};\n \t}\n \n-\tret = bind(sockets[0]);\n-\tif (ret)\n-\t\treturn ret;\n+\tstd::array<UniqueFD, 2> socketFds{\n+\t\tUniqueFD(sockets[0]),\n+\t\tUniqueFD(sockets[1]),\n+\t};\n \n-\treturn sockets[1];\n+\tif (bind(std::move(socketFds[0])) < 0)\n+\t\treturn {};\n+\n+\treturn std::move(socketFds[1]);\n }\n \n /**\n@@ -118,13 +123,13 @@ int IPCUnixSocket::create()\n *\n * \\return 0 on success or a negative error code otherwise\n */\n-int IPCUnixSocket::bind(int fd)\n+int IPCUnixSocket::bind(UniqueFD fd)\n {\n \tif (isBound())\n \t\treturn -EINVAL;\n \n-\tfd_ = fd;\n-\tnotifier_ = new EventNotifier(fd_, EventNotifier::Read);\n+\tfd_ = std::move(fd);\n+\tnotifier_ = new EventNotifier(fd_.get(), EventNotifier::Read);\n \tnotifier_->activated.connect(this, &IPCUnixSocket::dataNotifier);\n \n \treturn 0;\n@@ -143,9 +148,7 @@ void IPCUnixSocket::close()\n \tdelete notifier_;\n \tnotifier_ = nullptr;\n \n-\t::close(fd_);\n-\n-\tfd_ = -1;\n+\tfd_.reset();\n \theaderReceived_ = false;\n }\n \n@@ -155,7 +158,7 @@ void IPCUnixSocket::close()\n */\n bool IPCUnixSocket::isBound() const\n {\n-\treturn fd_ != -1;\n+\treturn fd_.isValid();\n }\n \n /**\n@@ -182,7 +185,7 @@ int IPCUnixSocket::send(const Payload &payload)\n \tif (!hdr.data && !hdr.fds)\n \t\treturn -EINVAL;\n \n-\tret = ::send(fd_, &hdr, sizeof(hdr), 0);\n+\tret = ::send(fd_.get(), &hdr, sizeof(hdr), 0);\n \tif (ret < 0) {\n \t\tret = -errno;\n \t\tLOG(IPCUnixSocket, Error)\n@@ -263,7 +266,7 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n \tif (fds)\n \t\tmemcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t));\n \n-\tif (sendmsg(fd_, &msg, 0) < 0) {\n+\tif (sendmsg(fd_.get(), &msg, 0) < 0) {\n \t\tint ret = -errno;\n \t\tLOG(IPCUnixSocket, Error)\n \t\t\t<< \"Failed to sendmsg: \" << strerror(-ret);\n@@ -297,7 +300,7 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,\n \tmsg.msg_controllen = cmsg->cmsg_len;\n \tmsg.msg_flags = 0;\n \n-\tif (recvmsg(fd_, &msg, 0) < 0) {\n+\tif (recvmsg(fd_.get(), &msg, 0) < 0) {\n \t\tint ret = -errno;\n \t\tif (ret != -EAGAIN)\n \t\t\tLOG(IPCUnixSocket, Error)\n@@ -317,7 +320,7 @@ void IPCUnixSocket::dataNotifier()\n \n \tif (!headerReceived_) {\n \t\t/* Receive the header. */\n-\t\tret = ::recv(fd_, &header_, sizeof(header_), 0);\n+\t\tret = ::recv(fd_.get(), &header_, sizeof(header_), 0);\n \t\tif (ret < 0) {\n \t\t\tret = -errno;\n \t\t\tLOG(IPCUnixSocket, Error)\n@@ -333,7 +336,7 @@ void IPCUnixSocket::dataNotifier()\n \t * readyRead signal. The notifier will be reenabled by the receive()\n \t * function.\n \t */\n-\tstruct pollfd fds = { fd_, POLLIN, 0 };\n+\tstruct pollfd fds = { fd_.get(), POLLIN, 0 };\n \tret = poll(&fds, 1, 0);\n \tif (ret < 0)\n \t\treturn;\ndiff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\nindex 7270bf4d2fe7..414f1bfc9d12 100644\n--- a/test/ipc/unixsocket.cpp\n+++ b/test/ipc/unixsocket.cpp\n@@ -52,9 +52,9 @@ public:\n \t\tipc_.readyRead.connect(this, &UnixSocketTestSlave::readyRead);\n \t}\n \n-\tint run(int fd)\n+\tint run(UniqueFD fd)\n \t{\n-\t\tif (ipc_.bind(fd)) {\n+\t\tif (ipc_.bind(std::move(fd))) {\n \t\t\tcerr << \"Failed to connect to IPC channel\" << endl;\n \t\t\treturn EXIT_FAILURE;\n \t\t}\n@@ -360,11 +360,11 @@ protected:\n \n \tint run()\n \t{\n-\t\tint slavefd = ipc_.create();\n-\t\tif (slavefd < 0)\n+\t\tUniqueFD slavefd = ipc_.create();\n+\t\tif (!slavefd.isValid())\n \t\t\treturn TestFail;\n \n-\t\tif (slaveStart(slavefd)) {\n+\t\tif (slaveStart(slavefd.release())) {\n \t\t\tcerr << \"Failed to start slave\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -496,9 +496,9 @@ private:\n int main(int argc, char **argv)\n {\n \tif (argc == 2) {\n-\t\tint ipcfd = std::stoi(argv[1]);\n+\t\tUniqueFD ipcfd = UniqueFD(std::stoi(argv[1]));\n \t\tUnixSocketTestSlave slave;\n-\t\treturn slave.run(ipcfd);\n+\t\treturn slave.run(std::move(ipcfd));\n \t}\n \n \treturn UnixSocketTest().execute();\ndiff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp\nindex ab5d25572d83..178ee1870056 100644\n--- a/test/ipc/unixsocket_ipc.cpp\n+++ b/test/ipc/unixsocket_ipc.cpp\n@@ -49,9 +49,9 @@ public:\n \t\tipc_.readyRead.connect(this, &UnixSocketTestIPCSlave::readyRead);\n \t}\n \n-\tint run(int fd)\n+\tint run(UniqueFD fd)\n \t{\n-\t\tif (ipc_.bind(fd)) {\n+\t\tif (ipc_.bind(std::move(fd))) {\n \t\t\tcerr << \"Failed to connect to IPC channel\" << endl;\n \t\t\treturn EXIT_FAILURE;\n \t\t}\n@@ -222,9 +222,9 @@ int main(int argc, char **argv)\n {\n \t/* IPCPipeUnixSocket passes IPA module path in argv[1] */\n \tif (argc == 3) {\n-\t\tint ipcfd = std::stoi(argv[2]);\n+\t\tUniqueFD ipcfd = UniqueFD(std::stoi(argv[2]));\n \t\tUnixSocketTestIPCSlave slave;\n-\t\treturn slave.run(ipcfd);\n+\t\treturn slave.run(std::move(ipcfd));\n \t}\n \n \treturn UnixSocketTestIPC().execute();\ndiff --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\nindex 764e7a3af63a..b65dc4cf31c5 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@@ -27,8 +27,9 @@\n #include <libcamera/logging.h>\n \n #include <libcamera/base/event_dispatcher.h>\n-#include <libcamera/base/thread.h>\n #include <libcamera/base/log.h>\n+#include <libcamera/base/thread.h>\n+#include <libcamera/base/unique_fd.h>\n \n #include \"libcamera/internal/camera_sensor.h\"\n #include \"libcamera/internal/control_serializer.h\"\n@@ -122,9 +123,9 @@ public:\n \t\t}\n \t}\n \n-\tint init(std::unique_ptr<IPAModule> &ipam, int socketfd)\n+\tint init(std::unique_ptr<IPAModule> &ipam, UniqueFD socketfd)\n \t{\n-\t\tif (socket_.bind(socketfd) < 0) {\n+\t\tif (socket_.bind(std::move(socketfd)) < 0) {\n \t\t\tLOG({{proxy_worker_name}}, Error)\n \t\t\t\t<< \"IPC socket binding failed\";\n \t\t\treturn EXIT_FAILURE;\n@@ -203,10 +204,10 @@ int main(int argc, char **argv)\n \t\treturn EXIT_FAILURE;\n \t}\n \n-\tint fd = std::stoi(argv[2]);\n+\tUniqueFD fd(std::stoi(argv[2]));\n \tLOG({{proxy_worker_name}}, Info)\n \t\t<< \"Starting worker for IPA module \" << argv[1]\n-\t\t<< \" with IPC fd = \" << fd;\n+\t\t<< \" with IPC fd = \" << fd.get();\n \n \tstd::unique_ptr<IPAModule> ipam = std::make_unique<IPAModule>(argv[1]);\n \tif (!ipam->isValid() || !ipam->load()) {\n@@ -228,7 +229,7 @@ int main(int argc, char **argv)\n \t}\n \n \t{{proxy_worker_name}} proxyWorker;\n-\tint ret = proxyWorker.init(ipam, fd);\n+\tint ret = proxyWorker.init(ipam, std::move(fd));\n \tif (ret < 0) {\n \t\tLOG({{proxy_worker_name}}, Error)\n \t\t\t<< \"Failed to initialize proxy worker\";\n", "prefixes": [ "libcamera-devel", "v3", "10/17" ] }