[libcamera-devel] [PATCH v3 04/17] libcamera: base: Introduce UniqueFD

[Hirokazu Honda]

Hi Laurent, thank you for the patch.

On Mon, Nov 29, 2021 at 8:58 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> From: Hirokazu Honda <hiroh at chromium.org>
>
> This introduces UniqueFD. It acts like unique_ptr to a file descriptor.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Looks good to me.

Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> ---
> Changes since v2:
>
> - Rename ScopedFD to UniqueFD
> - Inline most functions to allow compiler optimizations
> - Bring the API closer to unique_ptr<>
> - Add swap()
> - Documentation cleanups
> - Slip FileDescriptor constructor to separate patch
> - Fix isValid()
> ---
>  include/libcamera/base/meson.build |   1 +
>  include/libcamera/base/unique_fd.h |  69 ++++++++++++++++
>  src/libcamera/base/meson.build     |   1 +
>  src/libcamera/base/unique_fd.cpp   | 123 +++++++++++++++++++++++++++++
>  4 files changed, 194 insertions(+)
>  create mode 100644 include/libcamera/base/unique_fd.h
>  create mode 100644 src/libcamera/base/unique_fd.cpp
>
> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build
> index f73b00917409..cca374a769cc 100644
> --- a/include/libcamera/base/meson.build
> +++ b/include/libcamera/base/meson.build
> @@ -22,6 +22,7 @@ libcamera_base_headers = files([
>      'span.h',
>      'thread.h',
>      'timer.h',
> +    'unique_fd.h',
>      'utils.h',
>  ])
>
> diff --git a/include/libcamera/base/unique_fd.h b/include/libcamera/base/unique_fd.h
> new file mode 100644
> index 000000000000..ae4d96b75797
> --- /dev/null
> +++ b/include/libcamera/base/unique_fd.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * unique_fd.h - File descriptor wrapper that owns a file descriptor.
> + */
> +
> +#pragma once
> +
> +#include <utility>
> +
> +#include <libcamera/base/class.h>
> +#include <libcamera/base/compiler.h>
> +
> +namespace libcamera {
> +
> +class UniqueFD final
> +{
> +public:
> +       UniqueFD()
> +               : fd_(-1)
> +       {
> +       }
> +
> +       explicit UniqueFD(int fd)
> +               : fd_(fd)
> +       {
> +       }
> +
> +       UniqueFD(UniqueFD &&other)
> +               : fd_(other.release())
> +       {
> +       }
> +
> +       ~UniqueFD()
> +       {
> +               reset();
> +       }
> +
> +       UniqueFD &operator=(UniqueFD &&other)
> +       {
> +               reset(other.release());
> +               return *this;
> +       }
> +
> +       __nodiscard int release()
> +       {
> +               int fd = fd_;
> +               fd_ = -1;
> +               return fd;
> +       }
> +
> +       void reset(int fd = -1);
> +
> +       void swap(UniqueFD &other)
> +       {
> +               std::swap(fd_, other.fd_);
> +       }
> +
> +       int get() const { return fd_; }
> +       bool isValid() const { return fd_ >= 0; }
> +
> +private:
> +       LIBCAMERA_DISABLE_COPY(UniqueFD)
> +
> +       int fd_;
> +};
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> index d5254fda9cbf..b0d85bc19245 100644
> --- a/src/libcamera/base/meson.build
> +++ b/src/libcamera/base/meson.build
> @@ -17,6 +17,7 @@ libcamera_base_sources = files([
>      'signal.cpp',
>      'thread.cpp',
>      'timer.cpp',
> +    'unique_fd.cpp',
>      'utils.cpp',
>  ])
>
> diff --git a/src/libcamera/base/unique_fd.cpp b/src/libcamera/base/unique_fd.cpp
> new file mode 100644
> index 000000000000..83d6919cf623
> --- /dev/null
> +++ b/src/libcamera/base/unique_fd.cpp
> @@ -0,0 +1,123 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * unique_fd.cpp - File descriptor wrapper that owns a file descriptor
> + */
> +
> +#include <libcamera/base/unique_fd.h>
> +
> +#include <unistd.h>
> +#include <utility>
> +
> +#include <libcamera/base/log.h>
> +
> +/**
> + * \file base/unique_fd.h
> + * \brief File descriptor wrapper that owns a file descriptor
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(UniqueFD)
> +
> +/**
> + * \class UniqueFD
> + * \brief unique_ptr-like wrapper for a file descriptor
> + *
> + * The UniqueFD is a wrapper that owns and manages the lifetime of a file
> + * descriptor. It is constructed from a numerical file descriptor, and takes
> + * over its ownership. The file descriptor is closed when the UniqueFD is
> + * destroyed, or when it is assigned another file descriptor with operator=()
> + * or reset().
> + */
> +
> +/**
> + * \fn UniqueFD::UniqueFD()
> + * \brief Construct a UniqueFD that owns no file descriptor
> + */
> +
> +/**
> + * \fn UniqueFD::UniqueFD(int fd)
> + * \brief Construct a UniqueFD that owns \a fd
> + * \param[in] fd A file descriptor to manage
> + */
> +
> +/**
> + * \fn UniqueFD::UniqueFD(UniqueFD &&other)
> + * \brief Move constructor, create a UniqueFD by taking over \a other
> + * \param[in] other The other UniqueFD
> + *
> + * Create a UniqueFD by transferring ownership of the file descriptor owned by
> + * \a other. Upon return, the \a other UniqueFD is invalid.
> + */
> +
> +/**
> + * \fn UniqueFD::~UniqueFD()
> + * \brief Destroy the UniqueFD instance
> + *
> + * If a file descriptor is owned, it is closed.
> + */
> +
> +/**
> + * \fn UniqueFD::operator=(UniqueFD &&other)
> + * \brief Move assignment operator, replace a UniqueFD by taking over \a other
> + * \param[in] other The other UniqueFD
> + *
> + * If this UniqueFD owns a file descriptor, the file descriptor is closed
> + * first. The file descriptor is then replaced by the one of \a other. Upon
> + * return, \a other is invalid.
> + *
> + * \return A reference to this UniqueFD
> + */
> +
> +/**
> + * \fn UniqueFD::release()
> + * \brief Release ownership of the file descriptor without closing it
> + *
> + * This function releases and returns the owned file descriptor without closing
> + * it. The caller owns the returned value and must take care of handling its
> + * life time to avoid file descriptor leakages. Upon return this UniqueFD is
> + * invalid.
> + *
> + * \return The managed file descriptor, or -1 if no file descriptor was owned
> + */
> +
> +/**
> + * \brief Replace the managed file descriptor
> + * \param[in] fd The new file descriptor to manage
> + *
> + * Close the managed file descriptor, if any, and replace it with the new \a fd.
> + *
> + * Self-resetting (passing an \a fd already managed by this instance) is invalid
> + * and results in undefined behaviour.
> + */
> +void UniqueFD::reset(int fd)
> +{
> +       ASSERT(!isValid() || fd != fd_);
> +
> +       std::swap(fd, fd_);
> +
> +       if (fd >= 0)
> +               close(fd);
> +}
> +
> +/**
> + * \fn UniqueFD::swap(UniqueFD &other)
> + * \brief Swap the managed file descriptors with another UniqueFD
> + * \param[in] other Another UniqueFD to swap the file descriptor with
> + */
> +
> +/**
> + * \fn UniqueFD::get()
> + * \brief Retrieve the managed file descriptor
> + * \return The managed file descriptor, or -1 if no file descriptor is owned
> + */
> +
> +/**
> + * \fn UniqueFD::isValid()
> + * \brief Check if the UniqueFD owns a valid file descriptor
> + * \return True if the UniqueFD owns a valid file descriptor, false otherwise
> + */
> +
> +} /* namespace libcamera */
> --
> Regards,
>
> Laurent Pinchart
>