[libcamera-devel] [PATCH v4 03/22] libcamera: base: file_descriptor: Move inode() function to frame_buffer.cpp
Hirokazu Honda
hiroh at chromium.org
Tue Nov 30 06:15:58 CET 2021
Hi Laurent, thank you for the patch.
On Tue, Nov 30, 2021 at 12:38 PM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> The inode() function has always been a bit of an outcast in the
> FileDescriptor class, as it's not related to the core feature provided
> by FileDescriptor, a shared ownership wrapper around file descriptors.
> As it's only used in the FrameBuffer implementation, move it to
> frame_buffer.cpp as a static function.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> ---
> Changes since v3:
>
> - Move the inode function to frame_buffer.cpp
> ---
> include/libcamera/base/file.h | 2 ++
> include/libcamera/base/file_descriptor.h | 3 ---
> src/libcamera/base/file.cpp | 1 +
> src/libcamera/base/file_descriptor.cpp | 25 ---------------------
> src/libcamera/framebuffer.cpp | 28 ++++++++++++++++++++++--
> 5 files changed, 29 insertions(+), 30 deletions(-)
>
> diff --git a/include/libcamera/base/file.h b/include/libcamera/base/file.h
> index 55e8edd934d4..9b62871b8230 100644
> --- a/include/libcamera/base/file.h
> +++ b/include/libcamera/base/file.h
> @@ -20,6 +20,8 @@
>
> namespace libcamera {
>
> +class FileDescriptor;
> +
> class File
> {
> public:
> diff --git a/include/libcamera/base/file_descriptor.h b/include/libcamera/base/file_descriptor.h
> index 8d764f8b4a26..5d1594e80801 100644
> --- a/include/libcamera/base/file_descriptor.h
> +++ b/include/libcamera/base/file_descriptor.h
> @@ -8,7 +8,6 @@
> #pragma once
>
> #include <memory>
> -#include <sys/types.h>
>
> namespace libcamera {
>
> @@ -28,8 +27,6 @@ public:
> int fd() const { return fd_ ? fd_->fd() : -1; }
> FileDescriptor dup() const;
>
> - ino_t inode() const;
> -
> private:
> class Descriptor
> {
> diff --git a/src/libcamera/base/file.cpp b/src/libcamera/base/file.cpp
> index ae4be1f95fa6..a04dfacdc102 100644
> --- a/src/libcamera/base/file.cpp
> +++ b/src/libcamera/base/file.cpp
> @@ -14,6 +14,7 @@
> #include <sys/types.h>
> #include <unistd.h>
>
> +#include <libcamera/base/file_descriptor.h>
> #include <libcamera/base/log.h>
>
> /**
> diff --git a/src/libcamera/base/file_descriptor.cpp b/src/libcamera/base/file_descriptor.cpp
> index f5f87c56eee8..98d4b4bfd24f 100644
> --- a/src/libcamera/base/file_descriptor.cpp
> +++ b/src/libcamera/base/file_descriptor.cpp
> @@ -8,7 +8,6 @@
> #include <libcamera/base/file_descriptor.h>
>
> #include <string.h>
> -#include <sys/stat.h>
> #include <sys/types.h>
> #include <unistd.h>
> #include <utility>
> @@ -223,30 +222,6 @@ FileDescriptor FileDescriptor::dup() const
> return FileDescriptor(fd());
> }
>
> -/**
> - * \brief Retrieve the file descriptor inode
> - *
> - * \todo Should this move to the File class ?
> - *
> - * \return The file descriptor inode on success, or 0 on error
> - */
> -ino_t FileDescriptor::inode() const
> -{
> - if (!isValid())
> - return 0;
> -
> - struct stat st;
> - int ret = fstat(fd_->fd(), &st);
> - if (ret < 0) {
> - ret = -errno;
> - LOG(FileDescriptor, Fatal)
> - << "Failed to fstat() fd: " << strerror(-ret);
> - return 0;
> - }
> -
> - return st.st_ino;
> -}
> -
> FileDescriptor::Descriptor::Descriptor(int fd, bool duplicate)
> {
> if (!duplicate) {
> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
> index 337ea1155a38..b1635f49b68a 100644
> --- a/src/libcamera/framebuffer.cpp
> +++ b/src/libcamera/framebuffer.cpp
> @@ -8,6 +8,9 @@
> #include <libcamera/framebuffer.h>
> #include "libcamera/internal/framebuffer.h"
>
> +#include <sys/stat.h>
> +
> +#include <libcamera/base/file.h>
> #include <libcamera/base/log.h>
>
> /**
> @@ -207,6 +210,27 @@ FrameBuffer::Private::Private()
> * \brief The plane length in bytes
> */
>
> +namespace {
> +
> +ino_t fileDescriptorInode(const FileDescriptor &fd)
> +{
> + if (!fd.isValid())
> + return 0;
> +
> + struct stat st;
> + int ret = fstat(fd.fd(), &st);
> + if (ret < 0) {
> + ret = -errno;
> + LOG(Buffer, Fatal)
> + << "Failed to fstat() fd: " << strerror(-ret);
> + return 0;
> + }
> +
> + return st.st_ino;
> +}
> +
> +} /* namespace */
> +
> /**
> * \brief Construct a FrameBuffer with an array of planes
> * \param[in] planes The frame memory planes
> @@ -236,8 +260,8 @@ FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
> */
> if (plane.fd.fd() != planes_[0].fd.fd()) {
> if (!inode)
> - inode = planes_[0].fd.inode();
> - if (plane.fd.inode() != inode) {
> + inode = fileDescriptorInode(planes_[0].fd);
> + if (fileDescriptorInode(plane.fd) != inode) {
> isContiguous = false;
> break;
> }
> --
> Regards,
>
> Laurent Pinchart
>
More information about the libcamera-devel
mailing list