[libcamera-devel] [PATCH 4/4] libcamera: file: Create the file on open() if it doesn't exist
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jul 13 13:57:41 CEST 2020
Hi Kieran,
On Sun, Jul 12, 2020 at 08:51:41PM +0100, Kieran Bingham wrote:
> Hi Laurent,
>
> On 12/07/2020 15:44, Laurent Pinchart wrote:
> > When a file is opened in WriteOnly or ReadWrite mode, create it if it
> > doesn't exist.
>
> Indeed, this solved my issue when using this series before.
>
> Thanks!
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > src/libcamera/file.cpp | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp
> > index 8bd109090919..77701666e438 100644
> > --- a/src/libcamera/file.cpp
> > +++ b/src/libcamera/file.cpp
> > @@ -148,8 +148,9 @@ bool File::exists() const
> > * \param[in] mode The open mode
> > *
> > * This function opens the file specified by fileName() in \a mode. If the file
> > - * doesn't exist and the mode is WriteOnly or ReadWrite, this
> > - * function will attempt to create the file.
> > + * doesn't exist and the mode is WriteOnly or ReadWrite, this function will
> > + * attempt to create the file with initial permissions set to 0666 (modified by
> > + * the process' umask).
> > *
> > * The error() status is updated.
> > *
> > @@ -163,8 +164,10 @@ bool File::open(File::OpenMode mode)
> > }
> >
> > int flags = (mode & ReadWrite) - 1;
> > + if (mode & WriteOnly)
>
> This looks a bit awkward, as it's actually WriteOnly or ReadWrite (i.e.
> it's not 'Write .. Only') because we know the bit will be set in both
> cases ... but that is detailed above in the function comment anyway, so
> this is fine, and I'm sure if anyone was looking they'd figure out it
> works for RW files too :)
>
> In otherwords, it's only the use of the word 'only' in the enum that's
> awkward, but not something worthy of a change of the enum.
Should we instead have File::OpenMode::Read and File::OpenMode::Write,
and left the user combine those ? If I had been creating this from
scratch that's likely what I would have done, but ReadOnly, WriteOnly
and ReadWrite are established terms.
> > + flags |= O_CREAT;
> >
> > - fd_ = ::open(name_.c_str(), flags);
> > + fd_ = ::open(name_.c_str(), flags, 0666);
> > if (fd_ < 0) {
> > error_ = -errno;
> > return false;
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list