From patchwork Wed Apr 14 09:17:03 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 11925 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 0D556BD1F6 for ; Wed, 14 Apr 2021 09:17:13 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2FC3A6880E; Wed, 14 Apr 2021 11:17:12 +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="Nh3xrVM6"; dkim-atps=neutral Received: from mail-pf1-x42d.google.com (mail-pf1-x42d.google.com [IPv6:2607:f8b0:4864:20::42d]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5E127602CA for ; Wed, 14 Apr 2021 11:17:11 +0200 (CEST) Received: by mail-pf1-x42d.google.com with SMTP id n38so13339357pfv.2 for ; Wed, 14 Apr 2021 02:17:11 -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:mime-version :content-transfer-encoding; bh=pI6W/9TVrbGWD814nS76WXabkbM3bTfW1eqYhsUOaes=; b=Nh3xrVM6FXUzI+ffzKZPNNwzACF9LkPDzHjMK1WKtJgN1JsLc6KQHNe0Q95gab+eZ/ A/8nxmwHF+hVlNLyD/j1xsb9gSmtppH6dhhHHuM+5YgjEVo1oDFyrmymWLpIKrJAdU4h S7GaBmoEc+/mKUsOIvxS+l0ZLPnhCTCgxsYVg= 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:mime-version :content-transfer-encoding; bh=pI6W/9TVrbGWD814nS76WXabkbM3bTfW1eqYhsUOaes=; b=DatnZ0sG4o+yPQ4l3XEahJNvd8OARD6cdvOmlEaT8Df+lsrKWordXdlJu3PGAAebhR d7S2E1ts7tfPw6liAeV8jXnMzOjydROaXM8k6iTuVv4mYNKyxFxAp2meuP5EaTa1YMuo 2EtxwRb/VKj97yjFoYZHpKhRswSkyC7KFfLwL9Uve+cnBjObmxDjJgXUgvnIMbEMNZlv b+W2L/hCO+L1eN9jYGNfJQb9MaE+kb5oW2jMKZPlpuA12YY9+qXu+MnWMH6oJy5HZLHS 4lhxFtwnT1x4hquKMstYOQ4yDgSUgYXn6GZsu7IpMBk4eembdtrukyeu//vFn3g7JKGO QNSg== X-Gm-Message-State: AOAM533t6K5ATmtADSsHTsjyV+n6UJAzecdD8c4FKthEXTkREM6LebHx ryaTLzzbJSvJzlf5Mo78Kq9FjwcZ7ylOoQ== X-Google-Smtp-Source: ABdhPJyarJogO7eaOblcgp2I6HackxL2/yD368Lg+mr8VWARCdTz9LXBFBbqwCscrE7eI4Km5du0YQ== X-Received: by 2002:a63:1352:: with SMTP id 18mr35297018pgt.11.1618391829600; Wed, 14 Apr 2021 02:17:09 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:2:84f5:7981:dfbe:8f02]) by smtp.gmail.com with ESMTPSA id g24sm14497298pgn.18.2021.04.14.02.17.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Apr 2021 02:17:09 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Wed, 14 Apr 2021 18:17:03 +0900 Message-Id: <20210414091703.3207932-1-hiroh@chromium.org> X-Mailer: git-send-email 2.31.1.295.g9ea45b61b8-goog MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH] [WIP] libcamera: FileDescriptor: Make FileDescriptor own a fd 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" Multiple FileDescriptors can share the same file descriptor. It leads that the ownership of a fd managed by the class is unclear. This makes FileDescriptor own a fd, similar to std::unique_ptr. Signed-off-by: Hirokazu Honda --- include/libcamera/file_descriptor.h | 26 ++----- src/libcamera/file_descriptor.cpp | 112 ++++------------------------ 2 files changed, 22 insertions(+), 116 deletions(-) diff --git a/include/libcamera/file_descriptor.h b/include/libcamera/file_descriptor.h index d514aac7..aaaf7f10 100644 --- a/include/libcamera/file_descriptor.h +++ b/include/libcamera/file_descriptor.h @@ -13,34 +13,22 @@ namespace libcamera { class FileDescriptor final { + static constexpr int kInvalidFd = -1; public: - explicit FileDescriptor(const int &fd = -1); - explicit FileDescriptor(int &&fd); - FileDescriptor(const FileDescriptor &other); + explicit FileDescriptor(const int fd = kInvalidFd); FileDescriptor(FileDescriptor &&other); ~FileDescriptor(); - FileDescriptor &operator=(const FileDescriptor &other); FileDescriptor &operator=(FileDescriptor &&other); + FileDescriptor &operator=(const FileDescriptor &other) = delete; + FileDescriptor(const FileDescriptor &other) = delete; - bool isValid() const { return fd_ != nullptr; } - int fd() const { return fd_ ? fd_->fd() : -1; } + bool isValid() const { return fd_ != kInvalidFd; } + int fd() const { return fd_; } FileDescriptor dup() const; private: - class Descriptor - { - public: - Descriptor(int fd, bool duplicate); - ~Descriptor(); - - int fd() const { return fd_; } - - private: - int fd_; - }; - - std::shared_ptr fd_; + int fd_; }; } /* namespace libcamera */ diff --git a/src/libcamera/file_descriptor.cpp b/src/libcamera/file_descriptor.cpp index 8b505ed3..5757aca0 100644 --- a/src/libcamera/file_descriptor.cpp +++ b/src/libcamera/file_descriptor.cpp @@ -7,6 +7,7 @@ #include +#include #include #include #include @@ -35,13 +36,7 @@ LOG_DEFINE_CATEGORY(FileDescriptor) * When constructed from a numerical file descriptor, the FileDescriptor * instance either duplicates or takes over the file descriptor: * - * - The FileDescriptor(const int &) constructor duplicates the numerical file - * descriptor and wraps the duplicate in a Descriptor. The caller is - * responsible for closing the original file descriptor, and the value - * returned by fd() will be different from the value passed to the - * constructor. - * - * - The FileDescriptor(int &&) constructor takes over the numerical file + * - The FileDescriptor(const int &) constructor takes over the numerical file * descriptor and wraps it in a Descriptor. The caller shall not touch the * original file descriptor once the function returns, and the value returned * by fd() will be identical to the value passed to the constructor. @@ -70,56 +65,9 @@ LOG_DEFINE_CATEGORY(FileDescriptor) * If the \a fd is negative, the FileDescriptor is constructed as invalid and * the fd() method will return -1. */ -FileDescriptor::FileDescriptor(const int &fd) -{ - if (fd < 0) - return; - - fd_ = std::make_shared(fd, true); - if (fd_->fd() < 0) - fd_.reset(); -} -/** - * \brief Create a FileDescriptor taking ownership of a given \a fd - * \param[in] fd File descriptor - * - * Construct a FileDescriptor from a numerical file descriptor by taking - * ownership of the \a fd. The original \a fd is set to -1 and shall not be - * touched by the caller anymore. In particular, the caller shall not close the - * original \a fd manually. The duplicated file descriptor will be closed - * automatically when all FileDescriptor instances that reference it are - * destroyed. - * - * If the \a fd is negative, the FileDescriptor is constructed as invalid and - * the fd() method will return -1. - */ -FileDescriptor::FileDescriptor(int &&fd) -{ - if (fd < 0) - return; - - fd_ = std::make_shared(fd, false); - /* - * The Descriptor constructor can't have failed here, as it took over - * the fd without duplicating it. Just set the original fd to -1 to - * implement move semantics. - */ - fd = -1; -} - -/** - * \brief Copy constructor, create a FileDescriptor from a copy of \a other - * \param[in] other The other FileDescriptor - * - * Copying a FileDescriptor implicitly shares ownership of the wrapped file - * descriptor. The original FileDescriptor is left untouched, and the caller is - * responsible for destroying it when appropriate. The wrapped file descriptor - * will be closed automatically when all FileDescriptor instances that - * reference it are destroyed. - */ -FileDescriptor::FileDescriptor(const FileDescriptor &other) - : fd_(other.fd_) +FileDescriptor::FileDescriptor(const int fd) + : fd_(fd) { } @@ -134,8 +82,9 @@ FileDescriptor::FileDescriptor(const FileDescriptor &other) * reference it are destroyed. */ FileDescriptor::FileDescriptor(FileDescriptor &&other) - : fd_(std::move(other.fd_)) + : fd_(other.fd_) { + other.fd_ = -1; } /** @@ -147,27 +96,8 @@ FileDescriptor::FileDescriptor(FileDescriptor &&other) */ FileDescriptor::~FileDescriptor() { -} - -/** - * \brief Copy assignment operator, replace the wrapped file descriptor with a - * copy of \a other - * \param[in] other The other FileDescriptor - * - * Copying a FileDescriptor creates a new reference to the wrapped file - * descriptor owner by \a other. If \a other is invalid, *this will also be - * invalid. The original FileDescriptor is left untouched, and the caller is - * responsible for destroying it when appropriate. The wrapped file descriptor - * will be closed automatically when all FileDescriptor instances that - * reference it are destroyed. - * - * \return A reference to this FileDescriptor - */ -FileDescriptor &FileDescriptor::operator=(const FileDescriptor &other) -{ - fd_ = other.fd_; - - return *this; + if (isValid()) + close(fd_); } /** @@ -186,7 +116,7 @@ FileDescriptor &FileDescriptor::operator=(const FileDescriptor &other) */ FileDescriptor &FileDescriptor::operator=(FileDescriptor &&other) { - fd_ = std::move(other.fd_); + std::swap(fd_, other.fd_); return *this; } @@ -218,29 +148,17 @@ FileDescriptor &FileDescriptor::operator=(FileDescriptor &&other) */ FileDescriptor FileDescriptor::dup() const { - return FileDescriptor(fd()); -} + if (!isValid()) + return FileDescriptor(); -FileDescriptor::Descriptor::Descriptor(int fd, bool duplicate) -{ - if (!duplicate) { - fd_ = fd; - return; - } - - /* Failing to dup() a fd should not happen and is fatal. */ - fd_ = ::dup(fd); - if (fd_ == -1) { + int dupFd = ::dup(fd); + if (dupFd == -1) { int ret = -errno; LOG(FileDescriptor, Fatal) << "Failed to dup() fd: " << strerror(-ret); + return FileDescriptor(); } -} -FileDescriptor::Descriptor::~Descriptor() -{ - if (fd_ != -1) - close(fd_); + return FileDescriptor(dupFd); } - } /* namespace libcamera */