[libcamera-devel] [PATCH v3 06/17] libcamera: base: file_descriptor: Add constructor from UniqueFD

[Jacopo Mondi]

Hi Laurent,

On Mon, Nov 29, 2021 at 06:43:21PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Nov 29, 2021 at 04:08:24PM +0100, Jacopo Mondi wrote:
> > On Mon, Nov 29, 2021 at 01:57:41AM +0200, Laurent Pinchart wrote:
> > > From: Hirokazu Honda <hiroh at chromium.org>
> > >
> > > Add a FileDescriptor constructor that takes a UniqueFD, transfering
> > > ownership of the file descriptor to the FileDescriptor.
> >
> > Is this a good idea ? Can shared_ptr<>(unique_ptr<>) ?
>
> For the second question, yes, see constructor 13 in
> https://en.cppreference.com/w/cpp/memory/shared_ptr/shared_ptr. For the
> first question, I think so, but we can discuss it :-)
>

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


> > > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > > Changes since v2:
> > >
> > > - Pass UniqueFD by value
> > > ---
> > >  include/libcamera/base/file_descriptor.h |  3 +++
> > >  src/libcamera/base/file_descriptor.cpp   | 13 +++++++++++++
> > >  2 files changed, 16 insertions(+)
> > >
> > > diff --git a/include/libcamera/base/file_descriptor.h b/include/libcamera/base/file_descriptor.h
> > > index 5d1594e80801..74292eba04f5 100644
> > > --- a/include/libcamera/base/file_descriptor.h
> > > +++ b/include/libcamera/base/file_descriptor.h
> > > @@ -11,11 +11,14 @@
> > >
> > >  namespace libcamera {
> > >
> > > +class UniqueFD;
> > > +
> > >  class FileDescriptor final
> > >  {
> > >  public:
> > >  	explicit FileDescriptor(const int &fd = -1);
> > >  	explicit FileDescriptor(int &&fd);
> > > +	explicit FileDescriptor(UniqueFD fd);
> > >  	FileDescriptor(const FileDescriptor &other);
> > >  	FileDescriptor(FileDescriptor &&other);
> > >  	~FileDescriptor();
> > > diff --git a/src/libcamera/base/file_descriptor.cpp b/src/libcamera/base/file_descriptor.cpp
> > > index 98d4b4bfd24f..da696b2501cd 100644
> > > --- a/src/libcamera/base/file_descriptor.cpp
> > > +++ b/src/libcamera/base/file_descriptor.cpp
> > > @@ -13,6 +13,7 @@
> > >  #include <utility>
> > >
> > >  #include <libcamera/base/log.h>
> > > +#include <libcamera/base/unique_fd.h>
> > >
> > >  /**
> > >   * \file base/file_descriptor.h
> > > @@ -109,6 +110,18 @@ FileDescriptor::FileDescriptor(int &&fd)
> > >  	fd = -1;
> > >  }
> > >
> > > +/**
> > > + * \brief Create a FileDescriptor taking ownership of a given UniqueFD \a fd
> > > + * \param[in] fd UniqueFD
> > > + *
> > > + * Construct a FileDescriptor from UniqueFD by taking ownership of the \a fd.
> > > + * The original \a fd becomes invalid.
> > > + */
> > > +FileDescriptor::FileDescriptor(UniqueFD fd)
> > > +	: FileDescriptor(fd.release())
> > > +{
> > > +}
> > > +
> > >  /**
> > >   * \brief Copy constructor, create a FileDescriptor from a copy of \a other
> > >   * \param[in] other The other FileDescriptor
>
> --
> Regards,
>
> Laurent Pinchart