From patchwork Thu Jun 10 07:50:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 12541 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 2CD0EBD78E for ; Thu, 10 Jun 2021 07:50:49 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DDC9968931; Thu, 10 Jun 2021 09:50:48 +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="TOTW+fMc"; dkim-atps=neutral Received: from mail-pl1-x62e.google.com (mail-pl1-x62e.google.com [IPv6:2607:f8b0:4864:20::62e]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8817968940 for ; Thu, 10 Jun 2021 09:50:47 +0200 (CEST) Received: by mail-pl1-x62e.google.com with SMTP id e1so534519pld.13 for ; Thu, 10 Jun 2021 00:50:47 -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=L/ATuyWRbMv4FyvQrPLRiCOVFfyVOn3wfjrXN4gF/64=; b=TOTW+fMc3aSmxTDzaEmG7gZDduOLmW1Wz2JNRXsa9jTwRqbz94E6ht/Fh2p2T+agEE mH1lbET1qsLX2q2adOs80R3bIv75y9m0wPhgTGAEr6RlEFbHPZOCshZ4Kv89hn571wJe aHqlBhtOnfdJTHFyXSBi5Ucx9ne0cd12uwibo= 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=L/ATuyWRbMv4FyvQrPLRiCOVFfyVOn3wfjrXN4gF/64=; b=a0+tCTftwABmOp4IR1FKXKtJpffQxedwK4htZlIRnjlwUnEmScqxqyl0UQhFSz3uRJ pR4ELDU+1hzTS3Z4bEMZYiii1Q9h3JYzxCZWZnVqi+cM0GsN6krhjNz/4nd69Vt8JRr4 7YsbFpGMawoRa2xNp2uxSy/+6pu083xTFo4jN+79bF/ZReBs1rdVCOkVyilKnSVsd+yA Tl99aFPgHNct/qdMYZaV6YrBZ07yw1vaqZNziuZO8yztmBvBUBUPLec/XkN+TdcNJ0AS P6iGsee5m6PndUP6Vc8JU/IjCKFz/x7bO51YKYww5DaKoFhv9UoEezXuAEppSjiKYZcH 840A== X-Gm-Message-State: AOAM532FbdKQUtWqn9qG5Z9snQXtd0IR0FJCpcjbHi45gvYL1GXADuhQ qn0LiZI7WzO97vHOMQ/BP6HIXkBIQoXlbg== X-Google-Smtp-Source: ABdhPJygdFXsHxtmydDPEFYK1k3XL0ilkOOcQGIG/ghAuWhpsUlLycGetUK3WSk9NFKSRWAsVNJOxw== X-Received: by 2002:a17:90a:fd05:: with SMTP id cv5mr2007108pjb.24.1623311445822; Thu, 10 Jun 2021 00:50:45 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:203:98e0:b356:1c8a:25d4]) by smtp.gmail.com with ESMTPSA id d66sm1565161pfa.32.2021.06.10.00.50.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Jun 2021 00:50:45 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Thu, 10 Jun 2021 16:50:25 +0900 Message-Id: <20210610075027.523672-9-hiroh@chromium.org> X-Mailer: git-send-email 2.32.0.rc1.229.g3e70b5a671-goog In-Reply-To: <20210610075027.523672-1-hiroh@chromium.org> References: <20210610075027.523672-1-hiroh@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 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..c620992b 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; }