From patchwork Thu Apr 15 08:38:41 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 11944 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 78089BD224 for ; Thu, 15 Apr 2021 08:39:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 24F1468821; Thu, 15 Apr 2021 10:39:02 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="QhCyrM8/"; dkim-atps=neutral Received: from mail-pf1-x42e.google.com (mail-pf1-x42e.google.com [IPv6:2607:f8b0:4864:20::42e]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 95D706880B for ; Thu, 15 Apr 2021 10:39:00 +0200 (CEST) Received: by mail-pf1-x42e.google.com with SMTP id m11so15592314pfc.11 for ; Thu, 15 Apr 2021 01:39:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=L2GJ5FdpixTlkZnE5WQQJ3Cr2SvhnXOuRKhPG46hJfw=; b=QhCyrM8/c+4VbnwAvyuohqHLc8ATnk7MJ2H5s1GWQEOY56DGWSfdYLFx5YqJ/k43am ubSlIHgJvAcotvcOwcxu3BocDhSJ+aRv/qsWLQkiJbQL/qEYp3UZD3LWYgDW9xghRvbI q0sXZS5VEErP/m1KBKBVurn0yEcuZbrts271Q= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=L2GJ5FdpixTlkZnE5WQQJ3Cr2SvhnXOuRKhPG46hJfw=; b=REfpOkyA5h6m+43NcPGUgkAWCnoXdvdyzLffKu8tXIR4cRzSopdJPCiB6bnsg39+DS MLdpx3YWZi0shUNyskaUcLZ8KKUt6mHYUBZMH+6Z4KxG+FXRg7ByYDYKEMMrQGqnCGNy AcJuTpK/GK0L5iE9c+lS0Sf5rK3yP+oxQhwnl7MxWhTmiX+11ZXcV6DqWp+gPXJYbTYU B6H6o9igVWOjhGiD0xesB4Dnd9nbc8jro6EPE6GCgO6Pzi5VG3JbNYQKafUtc2wTJp9Z ZguKSGVwGWEiQ4fGeevDUZ30CgPl4WSMccE364tpSQRRf3TXSN2K3EdhifgUSsuN4s6O doww== X-Gm-Message-State: AOAM532jrBwpGFBE7lb04tyk4AawSwSymuTx8aQBBUiylR3wg3DzJJYQ 0unNMZr9EzJUBBG9F+GI7mdWV+xIOOK11Q== X-Google-Smtp-Source: ABdhPJzlNSHIBAGITEaV+xsYWQHirm6xq22xFsXPyRZkKdH/C+w3qpdeAX+lHnx1JlAZ4V0uFTIG7Q== X-Received: by 2002:a63:fb15:: with SMTP id o21mr2461179pgh.337.1618475938994; Thu, 15 Apr 2021 01:38:58 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:2:84f5:7981:dfbe:8f02]) by smtp.gmail.com with ESMTPSA id 205sm1520258pfc.201.2021.04.15.01.38.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Apr 2021 01:38:58 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Thu, 15 Apr 2021 17:38:41 +0900 Message-Id: <20210415083843.3399502-8-hiroh@chromium.org> X-Mailer: git-send-email 2.31.1.368.gbe11c130af-goog In-Reply-To: <20210415083843.3399502-1-hiroh@chromium.org> References: <20210415083843.3399502-1-hiroh@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH 08/10] libcamera: IPCUnixSocket: Use ScopedFD for a file descriptor X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" IPCUnixSocket::create() creates two file descriptors. One of them is stored in IPCUnixSocket and the other is returned to a caller. This clarifies the ownership using ScopedFD. Signed-off-by: Hirokazu Honda --- include/libcamera/internal/ipc_unixsocket.h | 5 +-- src/libcamera/ipc_pipe_unixsocket.cpp | 8 ++--- src/libcamera/ipc_unixsocket.cpp | 40 ++++++++++----------- test/ipc/unixsocket.cpp | 6 ++-- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/include/libcamera/internal/ipc_unixsocket.h b/include/libcamera/internal/ipc_unixsocket.h index e871b650..c06fee47 100644 --- a/include/libcamera/internal/ipc_unixsocket.h +++ b/include/libcamera/internal/ipc_unixsocket.h @@ -12,6 +12,7 @@ #include #include +#include #include namespace libcamera { @@ -29,7 +30,7 @@ public: IPCUnixSocket(); ~IPCUnixSocket(); - int create(); + ScopedFD create(); int bind(int fd); void close(); bool isBound() const; @@ -50,7 +51,7 @@ private: void dataNotifier(EventNotifier *notifier); - int fd_; + ScopedFD fd_; bool headerReceived_; struct Header header_; EventNotifier *notifier_; diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp index db0e260f..1997643f 100644 --- a/src/libcamera/ipc_pipe_unixsocket.cpp +++ b/src/libcamera/ipc_pipe_unixsocket.cpp @@ -30,14 +30,14 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath, args.push_back(ipaModulePath); socket_ = std::make_unique(); - int fd = socket_->create(); - if (fd < 0) { + ScopedFD fd = socket_->create(); + if (!fd.isValid()) { LOG(IPCPipe, Error) << "Failed to create socket"; return; } socket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead); - args.push_back(std::to_string(fd)); - fds.push_back(fd); + args.push_back(std::to_string(fd.get())); + fds.push_back(fd.release()); proc_ = std::make_unique(); int ret = proc_->start(ipaProxyWorkerPath, args, fds); diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp index fdb359f7..2da43188 100644 --- a/src/libcamera/ipc_unixsocket.cpp +++ b/src/libcamera/ipc_unixsocket.cpp @@ -68,7 +68,7 @@ LOG_DEFINE_CATEGORY(IPCUnixSocket) */ IPCUnixSocket::IPCUnixSocket() - : fd_(-1), headerReceived_(false), notifier_(nullptr) + : headerReceived_(false), notifier_(nullptr) { } @@ -86,9 +86,9 @@ IPCUnixSocket::~IPCUnixSocket() * the remote process, where it can be used with IPCUnixSocket::bind() to bind * the remote side socket. * - * \return A file descriptor on success, negative error code on failure + * \return A file descriptor. It is valid on success or invalid otherwise. */ -int IPCUnixSocket::create() +ScopedFD IPCUnixSocket::create() { int sockets[2]; int ret; @@ -98,14 +98,13 @@ int IPCUnixSocket::create() ret = -errno; LOG(IPCUnixSocket, Error) << "Failed to create socket pair: " << strerror(-ret); - return ret; + return ScopedFD(); } - ret = bind(sockets[0]); - if (ret) - return ret; + if (bind(sockets[0]) < 0) + return ScopedFD(); - return sockets[1]; + return ScopedFD(sockets[1]); } /** @@ -116,15 +115,18 @@ int IPCUnixSocket::create() * by the file descriptor \a fd. The file descriptor is obtained from the * IPCUnixSocket::create() method. * - * \return 0 on success or a negative error code otherwise + * \return 0 on success or a negative error code otherwise. + * + * \todo This argument should be ScopedFD because bind() takes over the + * ownership. */ int IPCUnixSocket::bind(int fd) { if (isBound()) return -EINVAL; - fd_ = fd; - notifier_ = new EventNotifier(fd_, EventNotifier::Read); + fd_ = ScopedFD(fd); + notifier_ = new EventNotifier(fd_.get(), EventNotifier::Read); notifier_->activated.connect(this, &IPCUnixSocket::dataNotifier); return 0; @@ -143,9 +145,7 @@ void IPCUnixSocket::close() delete notifier_; notifier_ = nullptr; - ::close(fd_); - - fd_ = -1; + fd_.reset(); headerReceived_ = false; } @@ -155,7 +155,7 @@ void IPCUnixSocket::close() */ bool IPCUnixSocket::isBound() const { - return fd_ != -1; + return fd_.isValid(); } /** @@ -182,7 +182,7 @@ int IPCUnixSocket::send(const Payload &payload) if (!hdr.data && !hdr.fds) return -EINVAL; - ret = ::send(fd_, &hdr, sizeof(hdr), 0); + ret = ::send(fd_.get(), &hdr, sizeof(hdr), 0); if (ret < 0) { ret = -errno; LOG(IPCUnixSocket, Error) @@ -262,7 +262,7 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length, msg.msg_flags = 0; memcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t)); - if (sendmsg(fd_, &msg, 0) < 0) { + if (sendmsg(fd_.get(), &msg, 0) < 0) { int ret = -errno; LOG(IPCUnixSocket, Error) << "Failed to sendmsg: " << strerror(-ret); @@ -296,7 +296,7 @@ int IPCUnixSocket::recvData(void *buffer, size_t length, msg.msg_controllen = cmsg->cmsg_len; msg.msg_flags = 0; - if (recvmsg(fd_, &msg, 0) < 0) { + if (recvmsg(fd_.get(), &msg, 0) < 0) { int ret = -errno; if (ret != -EAGAIN) LOG(IPCUnixSocket, Error) @@ -315,7 +315,7 @@ void IPCUnixSocket::dataNotifier([[maybe_unused]] EventNotifier *notifier) if (!headerReceived_) { /* Receive the header. */ - ret = ::recv(fd_, &header_, sizeof(header_), 0); + ret = ::recv(fd_.get(), &header_, sizeof(header_), 0); if (ret < 0) { ret = -errno; LOG(IPCUnixSocket, Error) @@ -331,7 +331,7 @@ void IPCUnixSocket::dataNotifier([[maybe_unused]] EventNotifier *notifier) * readyRead signal. The notifier will be reenabled by the receive() * method. */ - struct pollfd fds = { fd_, POLLIN, 0 }; + struct pollfd fds = { fd_.get(), POLLIN, 0 }; ret = poll(&fds, 1, 0); if (ret < 0) return; diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp index 80157b34..9ca0467b 100644 --- a/test/ipc/unixsocket.cpp +++ b/test/ipc/unixsocket.cpp @@ -359,11 +359,11 @@ protected: int run() { - int slavefd = ipc_.create(); - if (slavefd < 0) + ScopedFD slavefd = ipc_.create(); + if (!slavefd.isValid()) return TestFail; - if (slaveStart(slavefd)) { + if (slaveStart(slavefd.release())) { cerr << "Failed to start slave" << endl; return TestFail; }