[libcamera-devel] [PATCH 4/4] libcamera: file: Create the file on open() if it doesn't exist

[Laurent Pinchart]

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