[libcamera-devel] [PATCH v4 03/22] libcamera: base: file_descriptor: Move inode() function to frame_buffer.cpp
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Nov 30 10:43:57 CET 2021
Hi Jacopo,
On Tue, Nov 30, 2021 at 08:50:11AM +0100, Jacopo Mondi wrote:
> On Tue, Nov 30, 2021 at 05:38:01AM +0200, Laurent Pinchart 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>
> > ---
> > 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;
> > +
>
> Is this needed as nothing is added to file.h ?
Absolutely not, it's an incorrect leftover.
> > 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>
>
> Same question
Same answer :-)
> > #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>
>
> Do you need file.h or rather file_descriptor.h for the new function ?
I really messed up this one, haven't I ? I'll fix it.
> > #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