[libcamera-devel] [PATCH v3 09/17] libcamera: file: Manage fd by UniqueFD

[Jacopo Mondi]

Hi Laurent

On Mon, Nov 29, 2021 at 01:57:44AM +0200, Laurent Pinchart wrote:
> From: Hirokazu Honda <hiroh at chromium.org>
>
> Manages the file descriptor owned by File by UniqueFD.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> ---
>  include/libcamera/base/file.h |  5 +++--
>  src/libcamera/base/file.cpp   | 25 ++++++++++++-------------
>  2 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/include/libcamera/base/file.h b/include/libcamera/base/file.h
> index 996751a7ab72..47769da7abc2 100644
> --- a/include/libcamera/base/file.h
> +++ b/include/libcamera/base/file.h
> @@ -17,6 +17,7 @@
>  #include <libcamera/base/class.h>
>  #include <libcamera/base/flags.h>
>  #include <libcamera/base/span.h>
> +#include <libcamera/base/unique_fd.h>
>
>  namespace libcamera {
>
> @@ -50,7 +51,7 @@ public:
>  	bool exists() const;
>
>  	bool open(OpenMode mode);
> -	bool isOpen() const { return fd_ != -1; }
> +	bool isOpen() const { return fd_.isValid(); }
>  	OpenMode openMode() const { return mode_; }
>  	void close();
>
> @@ -76,7 +77,7 @@ private:
>  	void unmapAll();
>
>  	std::string name_;
> -	int fd_;
> +	UniqueFD fd_;
>  	OpenMode mode_;
>
>  	int error_;
> diff --git a/src/libcamera/base/file.cpp b/src/libcamera/base/file.cpp
> index 7043f9461cf7..66c73c406198 100644
> --- a/src/libcamera/base/file.cpp
> +++ b/src/libcamera/base/file.cpp
> @@ -84,7 +84,7 @@ LOG_DEFINE_CATEGORY(File)
>   * before performing I/O operations.
>   */
>  File::File(const std::string &name)
> -	: name_(name), fd_(-1), mode_(OpenModeFlag::NotOpen), error_(0)
> +	: name_(name), mode_(OpenModeFlag::NotOpen), error_(0)
>  {
>  }
>
> @@ -95,7 +95,7 @@ File::File(const std::string &name)
>   * setFileName().
>   */
>  File::File()
> -	: fd_(-1), mode_(OpenModeFlag::NotOpen), error_(0)
> +	: mode_(OpenModeFlag::NotOpen), error_(0)
>  {
>  }
>
> @@ -178,8 +178,8 @@ bool File::open(File::OpenMode mode)
>  	if (mode & OpenModeFlag::WriteOnly)
>  		flags |= O_CREAT;
>
> -	fd_ = ::open(name_.c_str(), flags, 0666);
> -	if (fd_ < 0) {
> +	fd_ = UniqueFD(::open(name_.c_str(), flags, 0666));
> +	if (!fd_.isValid()) {
>  		error_ = -errno;
>  		return false;
>  	}
> @@ -210,11 +210,10 @@ bool File::open(File::OpenMode mode)
>   */
>  void File::close()
>  {
> -	if (fd_ == -1)
> +	if (!fd_.isValid())
>  		return;
>
> -	::close(fd_);
> -	fd_ = -1;
> +	fd_.reset();
>  	mode_ = OpenModeFlag::NotOpen;
>  }
>
> @@ -244,7 +243,7 @@ ssize_t File::size() const
>  		return -EINVAL;
>
>  	struct stat st;
> -	int ret = fstat(fd_, &st);
> +	int ret = fstat(fd_.get(), &st);
>  	if (ret < 0)
>  		return -errno;
>
> @@ -263,7 +262,7 @@ off_t File::pos() const
>  	if (!isOpen())
>  		return 0;
>
> -	return lseek(fd_, 0, SEEK_CUR);
> +	return lseek(fd_.get(), 0, SEEK_CUR);
>  }
>
>  /**
> @@ -277,7 +276,7 @@ off_t File::seek(off_t pos)
>  	if (!isOpen())
>  		return -EINVAL;
>
> -	off_t ret = lseek(fd_, pos, SEEK_SET);
> +	off_t ret = lseek(fd_.get(), pos, SEEK_SET);
>  	if (ret < 0)
>  		return -errno;
>
> @@ -309,7 +308,7 @@ ssize_t File::read(const Span<uint8_t> &data)
>
>  	/* Retry in case of interrupted system calls. */
>  	while (readBytes < data.size()) {
> -		ret = ::read(fd_, data.data() + readBytes,
> +		ret = ::read(fd_.get(), data.data() + readBytes,
>  			     data.size() - readBytes);
>  		if (ret <= 0)
>  			break;
> @@ -346,7 +345,7 @@ ssize_t File::write(const Span<const uint8_t> &data)
>
>  	/* Retry in case of interrupted system calls. */
>  	while (writtenBytes < data.size()) {
> -		ssize_t ret = ::write(fd_, data.data() + writtenBytes,
> +		ssize_t ret = ::write(fd_.get(), data.data() + writtenBytes,
>  				      data.size() - writtenBytes);
>  		if (ret <= 0)
>  			break;
> @@ -409,7 +408,7 @@ Span<uint8_t> File::map(off_t offset, ssize_t size, File::MapFlags flags)
>  	if (flags & MapFlag::Private)
>  		prot |= PROT_WRITE;
>
> -	void *map = mmap(NULL, size, prot, mmapFlags, fd_, offset);
> +	void *map = mmap(NULL, size, prot, mmapFlags, fd_.get(), offset);
>  	if (map == MAP_FAILED) {
>  		error_ = -errno;
>  		return {};
> --
> Regards,
>
> Laurent Pinchart
>