[libcamera-devel,03/11] libcamera: Add File helper class

3393 diff mbox series

The File helper class is a RAII wrapper for a file to manage the file
handle and memory-mapped regions.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/file.cpp            | 338 ++++++++++++++++++++++++++++++
 src/libcamera/include/file.h      |  69 ++++++
 src/libcamera/include/meson.build |   1 +
 src/libcamera/meson.build         |   1 +
 4 files changed, 409 insertions(+)
 create mode 100644 src/libcamera/file.cpp
 create mode 100644 src/libcamera/include/file.h

Hi Laurent,

Thanks for your work.

On 2020-04-04 04:56:16 +0300, Laurent Pinchart wrote:
> The File helper class is a RAII wrapper for a file to manage the file
> handle and memory-mapped regions.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/file.cpp            | 338 ++++++++++++++++++++++++++++++
>  src/libcamera/include/file.h      |  69 ++++++
>  src/libcamera/include/meson.build |   1 +
>  src/libcamera/meson.build         |   1 +
>  4 files changed, 409 insertions(+)
>  create mode 100644 src/libcamera/file.cpp
>  create mode 100644 src/libcamera/include/file.h
> 
> diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp
> new file mode 100644
> index 000000000000..23cea4aa3101
> --- /dev/null
> +++ b/src/libcamera/file.cpp
> @@ -0,0 +1,338 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * file.cpp - File I/O operations
> + */
> +
> +#include "file.h"
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include "log.h"
> +
> +/**
> + * \file file.h
> + * \brief File I/O operations
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(File);
> +
> +/**
> + * \class File
> + * \brief Interface for I/O operations on files
> + *
> + * The File class provides an interface to perform I/O operations on files. It
> + * wraps opening, closing and mapping files in memory, and handles the cleaning
> + * of allocated resources.
> + *
> + * File instances are usually constructed with a file name, but the name can be
> + * set later through the setFileName() function. Instances are not automatically
> + * opened when constructed, and shall be opened explictly with open().
> + *
> + * Files can be mapped to the process memory with map(). Mapped regions can be
> + * unmapped manually with munmap(), and are automatically unmapped when the File
> + * is destroyed.
> + */
> +
> +/**
> + * \enum File::MapFlag
> + * \brief Flags for the File::map() function
> + * \var File::MapNoOption
> + * \brief No option (used as default value)
> + * \var File::MapPrivate
> + * \brief The memory region is mapped as private, changes are not reflected in
> + * the file constents
> + */
> +
> +/**
> + * \enum File::OpenMode
> + * \brief Mode in which a file is opened
> + * \var File::NotOpen
> + * \brief The file is not open
> + * \var File::ReadOnly
> + * \brief The file is open for reading
> + * \var File::WriteOnly
> + * \brief The file is open for writing
> + * \var File::ReadWrite
> + * \brief The file is open for reading and writing
> + */
> +
> +/**
> + * \brief Construct a File to represent the file \a name
> + * \param[in] name The file name
> + *
> + * Upon construction the File object is closed and shall be opened with open()
> + * before performing I/O operations.
> + */
> +File::File(const std::string &name)
> +	: name_(name), fd_(-1), mode_(NotOpen), error_(0)
> +{
> +}
> +
> +/**
> + * \brief Construct a File without an associated name
> + *
> + * Before being used for any purpose, the file name shall be set with
> + * setFileName().
> + */
> +File::File()
> +	: fd_(-1), mode_(NotOpen), error_(0)
> +{
> +}
> +
> +/**
> + * \brief Destroy a File instance
> + *
> + * Any memory mapping associated with the File is unmapped, and the File is
> + * closed if it is open.
> + */
> +File::~File()
> +{
> +	for (const auto &map : maps_)
> +		munmap(map.first, map.second);
> +
> +	close();
> +}
> +
> +/**
> + * \fn const std::string &File::fileName() const
> + * \brief Retrieve the file name
> + * \return The file name
> + */
> +
> +/**
> + * \brief Set the name of the file
> + * \param[in] name The name of the file
> + *
> + * The \a name can contain an absolute path, a relative path or no path at all.
> + * Calling this function on an open file results in undefined behaviour.
> + */
> +void File::setFileName(const std::string &name)
> +{
> +	if (isOpen()) {
> +		LOG(File, Error)
> +			<< "Can't set file name on already open file " << name_;
> +		return;
> +	}
> +
> +	name_ = name;
> +}
> +
> +/**
> + * \brief Check if the file specified by fileName() exists
> + *
> + * This function checks if the file specified by fileName() exists. The File
> + * instance doesn't need to be open to check for file existence, and this
> + * function may return false even if the file is open, if it was deleted from
> + * the file system.
> + *
> + * \return True if the the file exists, false otherwise
> + */
> +bool File::exists() const
> +{
> +	return exists(name_);
> +}
> +
> +/**
> + * \brief Open the file in the given mode
> + * \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.

Shall the updated error status be mentioned here ?

> + *
> + * \return True on success, false otherwise
> + */
> +bool File::open(File::OpenMode mode)
> +{
> +	if (isOpen()) {
> +		LOG(File, Error) << "File " << name_ << " is already open";
> +		return false;
> +	}
> +
> +	int flags = (mode & ReadWrite) - 1;
> +
> +	fd_ = ::open(name_.c_str(), flags);
> +	if (fd_ < 0) {
> +		error_ = -errno;
> +		return false;
> +	}
> +
> +	mode_ = mode;
> +	error_ = 0;
> +	return true;
> +}
> +
> +/**
> + * \fn bool File::isOpen() const
> + * \brief Check if the file is open
> + * \return True if the file is open, false otherwise
> + */
> +
> +/**
> + * \fn OpenMode File::openMode() const
> + * \brief Retrieve the file open mode
> + * \return The file open mode
> + */
> +
> +/**
> + * \brief Close the file
> + *
> + * This function closes the File. If the File is not open, it performs no
> + * operation. Memory mappings created with map() are not destroyed when the
> + * file is closed.
> + */
> +void File::close()
> +{
> +	if (fd_ == -1)
> +		return;
> +
> +	::close(fd_);
> +	fd_ = -1;
> +	mode_ = NotOpen;
> +}
> +
> +/**
> + * \fn int File::error() const
> + * \brief Retrieve the file error status
> + *
> + * This function retrieves the error status from the last file open or I/O
> + * operation. The error status is a negative number as defined by errno.h. If
> + * no error occurred, this function returns 0.
> + *
> + * \return The file error status
> + */
> +
> +/**
> + * \brief Retrieve the file size
> + *
> + * This function retrieves the size of the file on the filesystem. The File
> + * instance shall be open to retrieve its size. The error() status is not
> + * modified, error codes are returned directly on failure.
> + *
> + * \return The file size in bytes on success, or a negative error code otherwise
> + */
> +ssize_t File::size() const
> +{
> +	if (!isOpen())
> +		return -EINVAL;
> +
> +	struct stat st;
> +	int ret = fstat(fd_, &st);
> +	if (ret < 0)
> +		return -errno;
> +
> +	return st.st_size;
> +}
> +
> +/**
> + * \brief Map a region of the file in the process memory
> + * \param[in] offset The region offset within the file
> + * \param[in] size The region sise
> + * \param[in] flags The mapping flags
> + *
> + * This function maps a region of \a size bytes of the file starting at \a
> + * offset into the process memory. The File instance shall be open, but may be
> + * closed after mapping the region. Mappings stay valid when the File is
> + * closed, and are destroyed automatically when the File is deleted.

I find this a bit dangerous. Would it not be confusing if we allowed a 
File to be used to open and map /foo/bar, then close it open and map 
/foo/baz and both mappings are removed when the single a signle File 
object is destroyed ?

> + *
> + * If \a size is a negative value, this function maps the region starting at \a
> + * offset until the end of the file.
> + *
> + * The mapping memory protection is controlled by the file open mode, unless \a
> + * flags contains MapPrivate in which case the region is mapped in read/write
> + * mode.
> + *
> + * The error() status is updated.
> + *
> + * \return The mapped memory on success, or an empty span otherwise
> + */
> +Span<uint8_t> File::map(off_t offset, ssize_t size, enum File::MapFlag flags)
> +{
> +	if (!isOpen()) {
> +		error_ = -EBADF;
> +		return {};
> +	}
> +
> +	if (size < 0) {
> +		size = File::size();
> +		if (size < 0) {
> +			error_ = size;
> +			return {};
> +		}
> +
> +		size -= offset;
> +	}
> +
> +	int mmapFlags = flags & MapPrivate ? MAP_PRIVATE : MAP_SHARED;
> +
> +	int prot = 0;
> +	if (mode_ & ReadOnly)
> +		prot |= PROT_READ;
> +	if (mode_ & WriteOnly)
> +		prot |= PROT_WRITE;
> +	if (flags & MapPrivate)
> +		prot |= PROT_WRITE;
> +
> +	void *map = mmap(NULL, size, prot, mmapFlags, fd_, offset);
> +	if (map == MAP_FAILED) {
> +		error_ = -errno;
> +		return {};
> +	}
> +
> +	maps_.emplace(map, size);
> +
> +	error_ = 0;
> +	return { static_cast<uint8_t *>(map), static_cast<size_t>(size) };
> +}
> +
> +/**
> + * \brief Unmap a region mapped with map()
> + * \param[in] addr The region address
> + *
> + * The error() status is updated.
> + *
> + * \return True on success, or false if an error occurs
> + */
> +bool File::unmap(uint8_t *addr)
> +{
> +	auto iter = maps_.find(static_cast<void *>(addr));
> +	if (iter == maps_.end()) {
> +		error_ = -ENOENT;
> +		return false;
> +	}
> +
> +	int ret = munmap(addr, iter->second);
> +	if (ret < 0) {
> +		error_ = -errno;
> +		return false;
> +	}
> +
> +	maps_.erase(iter);

I think error_ shall be set to 0 here right?

> +	return true;
> +}
> +
> +/**
> + * \brief Check if the file specified by \a name exists
> + * \param[in] name The file name
> + * \return True if the file exists, false otherwise
> + */
> +bool File::exists(const std::string &name)
> +{
> +	struct stat st;
> +	int ret = stat(name.c_str(), &st);
> +	if (ret < 0)
> +		return false;
> +
> +	return true;
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/include/file.h b/src/libcamera/include/file.h
> new file mode 100644
> index 000000000000..ea6f121cb6c5
> --- /dev/null
> +++ b/src/libcamera/include/file.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * file.h - File I/O operations
> + */
> +#ifndef __LIBCAMERA_FILE_H__
> +#define __LIBCAMERA_FILE_H__
> +
> +#include <map>
> +#include <string>
> +#include <sys/types.h>
> +
> +#include <libcamera/span.h>
> +
> +namespace libcamera {
> +
> +class File
> +{
> +public:
> +	enum MapFlag {
> +		MapNoOption = 0,
> +		MapPrivate = (1 << 0),
> +	};
> +
> +	enum OpenMode {
> +		NotOpen = 0,
> +		ReadOnly = (1 << 0),
> +		WriteOnly = (1 << 1),
> +		ReadWrite = ReadOnly | WriteOnly,
> +	};
> +
> +	File(const std::string &name);
> +	File();
> +	~File();
> +
> +	File(const File &) = delete;
> +	File &operator=(const File &) = delete;
> +
> +	const std::string &fileName() const { return name_; }
> +	void setFileName(const std::string &name);
> +	bool exists() const;
> +
> +	bool open(OpenMode mode);

The API seems a bit odd here.

When opening a file we are required to provide how to open it but not 
which file to open. Would this class be simplified if the file to open 
would also be provided as an argument to open instead of either to a 
constructor or setFileName() ?

> +	bool isOpen() const { return fd_ != -1; }
> +	OpenMode openMode() const { return mode_; }
> +	void close();
> +
> +	int error() const { return error_; }
> +	ssize_t size() const;
> +
> +	Span<uint8_t> map(off_t offset = 0, ssize_t size = -1,
> +			  MapFlag flags = MapNoOption);
> +	bool unmap(uint8_t *addr);

Would it make sens to pass the Span<uint8_t> returned from map as an 
argument to unmap? Reading the class definition suggest their is an 
unbalance between new fancy stuff and plain old data ;-)

> +
> +	static bool exists(const std::string &name);
> +
> +private:
> +	std::string name_;
> +	int fd_;
> +	OpenMode mode_;
> +
> +	int error_;
> +	std::map<void *, size_t> maps_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_FILE_H__ */
> diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build
> index 17e2bed93fba..921ed5a063cb 100644
> --- a/src/libcamera/include/meson.build
> +++ b/src/libcamera/include/meson.build
> @@ -8,6 +8,7 @@ libcamera_headers = files([
>      'device_enumerator_sysfs.h',
>      'device_enumerator_udev.h',
>      'event_dispatcher_poll.h',
> +    'file.h',
>      'formats.h',
>      'ipa_context_wrapper.h',
>      'ipa_manager.h',
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 87fa09cde63d..4f5c41678781 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -14,6 +14,7 @@ libcamera_sources = files([
>      'event_dispatcher.cpp',
>      'event_dispatcher_poll.cpp',
>      'event_notifier.cpp',
> +    'file.cpp',
>      'file_descriptor.cpp',
>      'formats.cpp',
>      'framebuffer_allocator.cpp',
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Niklas,

On Tue, Apr 07, 2020 at 10:19:34PM +0200, Niklas Söderlund wrote:
> On 2020-04-04 04:56:16 +0300, Laurent Pinchart wrote:
> > The File helper class is a RAII wrapper for a file to manage the file
> > handle and memory-mapped regions.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/file.cpp            | 338 ++++++++++++++++++++++++++++++
> >  src/libcamera/include/file.h      |  69 ++++++
> >  src/libcamera/include/meson.build |   1 +
> >  src/libcamera/meson.build         |   1 +
> >  4 files changed, 409 insertions(+)
> >  create mode 100644 src/libcamera/file.cpp
> >  create mode 100644 src/libcamera/include/file.h
> > 
> > diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp
> > new file mode 100644
> > index 000000000000..23cea4aa3101
> > --- /dev/null
> > +++ b/src/libcamera/file.cpp
> > @@ -0,0 +1,338 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * file.cpp - File I/O operations
> > + */
> > +
> > +#include "file.h"
> > +
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <sys/mman.h>
> > +#include <sys/stat.h>
> > +#include <sys/types.h>
> > +#include <unistd.h>
> > +
> > +#include "log.h"
> > +
> > +/**
> > + * \file file.h
> > + * \brief File I/O operations
> > + */
> > +
> > +namespace libcamera {
> > +
> > +LOG_DEFINE_CATEGORY(File);
> > +
> > +/**
> > + * \class File
> > + * \brief Interface for I/O operations on files
> > + *
> > + * The File class provides an interface to perform I/O operations on files. It
> > + * wraps opening, closing and mapping files in memory, and handles the cleaning
> > + * of allocated resources.
> > + *
> > + * File instances are usually constructed with a file name, but the name can be
> > + * set later through the setFileName() function. Instances are not automatically
> > + * opened when constructed, and shall be opened explictly with open().
> > + *
> > + * Files can be mapped to the process memory with map(). Mapped regions can be
> > + * unmapped manually with munmap(), and are automatically unmapped when the File
> > + * is destroyed.
> > + */
> > +
> > +/**
> > + * \enum File::MapFlag
> > + * \brief Flags for the File::map() function
> > + * \var File::MapNoOption
> > + * \brief No option (used as default value)
> > + * \var File::MapPrivate
> > + * \brief The memory region is mapped as private, changes are not reflected in
> > + * the file constents
> > + */
> > +
> > +/**
> > + * \enum File::OpenMode
> > + * \brief Mode in which a file is opened
> > + * \var File::NotOpen
> > + * \brief The file is not open
> > + * \var File::ReadOnly
> > + * \brief The file is open for reading
> > + * \var File::WriteOnly
> > + * \brief The file is open for writing
> > + * \var File::ReadWrite
> > + * \brief The file is open for reading and writing
> > + */
> > +
> > +/**
> > + * \brief Construct a File to represent the file \a name
> > + * \param[in] name The file name
> > + *
> > + * Upon construction the File object is closed and shall be opened with open()
> > + * before performing I/O operations.
> > + */
> > +File::File(const std::string &name)
> > +	: name_(name), fd_(-1), mode_(NotOpen), error_(0)
> > +{
> > +}
> > +
> > +/**
> > + * \brief Construct a File without an associated name
> > + *
> > + * Before being used for any purpose, the file name shall be set with
> > + * setFileName().
> > + */
> > +File::File()
> > +	: fd_(-1), mode_(NotOpen), error_(0)
> > +{
> > +}
> > +
> > +/**
> > + * \brief Destroy a File instance
> > + *
> > + * Any memory mapping associated with the File is unmapped, and the File is
> > + * closed if it is open.
> > + */
> > +File::~File()
> > +{
> > +	for (const auto &map : maps_)
> > +		munmap(map.first, map.second);
> > +
> > +	close();
> > +}
> > +
> > +/**
> > + * \fn const std::string &File::fileName() const
> > + * \brief Retrieve the file name
> > + * \return The file name
> > + */
> > +
> > +/**
> > + * \brief Set the name of the file
> > + * \param[in] name The name of the file
> > + *
> > + * The \a name can contain an absolute path, a relative path or no path at all.
> > + * Calling this function on an open file results in undefined behaviour.
> > + */
> > +void File::setFileName(const std::string &name)
> > +{
> > +	if (isOpen()) {
> > +		LOG(File, Error)
> > +			<< "Can't set file name on already open file " << name_;
> > +		return;
> > +	}
> > +
> > +	name_ = name;
> > +}
> > +
> > +/**
> > + * \brief Check if the file specified by fileName() exists
> > + *
> > + * This function checks if the file specified by fileName() exists. The File
> > + * instance doesn't need to be open to check for file existence, and this
> > + * function may return false even if the file is open, if it was deleted from
> > + * the file system.
> > + *
> > + * \return True if the the file exists, false otherwise
> > + */
> > +bool File::exists() const
> > +{
> > +	return exists(name_);
> > +}
> > +
> > +/**
> > + * \brief Open the file in the given mode
> > + * \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.
> 
> Shall the updated error status be mentioned here ?

Fixed.

> > + *
> > + * \return True on success, false otherwise
> > + */
> > +bool File::open(File::OpenMode mode)
> > +{
> > +	if (isOpen()) {
> > +		LOG(File, Error) << "File " << name_ << " is already open";
> > +		return false;
> > +	}
> > +
> > +	int flags = (mode & ReadWrite) - 1;
> > +
> > +	fd_ = ::open(name_.c_str(), flags);
> > +	if (fd_ < 0) {
> > +		error_ = -errno;
> > +		return false;
> > +	}
> > +
> > +	mode_ = mode;
> > +	error_ = 0;
> > +	return true;
> > +}
> > +
> > +/**
> > + * \fn bool File::isOpen() const
> > + * \brief Check if the file is open
> > + * \return True if the file is open, false otherwise
> > + */
> > +
> > +/**
> > + * \fn OpenMode File::openMode() const
> > + * \brief Retrieve the file open mode
> > + * \return The file open mode
> > + */
> > +
> > +/**
> > + * \brief Close the file
> > + *
> > + * This function closes the File. If the File is not open, it performs no
> > + * operation. Memory mappings created with map() are not destroyed when the
> > + * file is closed.
> > + */
> > +void File::close()
> > +{
> > +	if (fd_ == -1)
> > +		return;
> > +
> > +	::close(fd_);
> > +	fd_ = -1;
> > +	mode_ = NotOpen;
> > +}
> > +
> > +/**
> > + * \fn int File::error() const
> > + * \brief Retrieve the file error status
> > + *
> > + * This function retrieves the error status from the last file open or I/O
> > + * operation. The error status is a negative number as defined by errno.h. If
> > + * no error occurred, this function returns 0.
> > + *
> > + * \return The file error status
> > + */
> > +
> > +/**
> > + * \brief Retrieve the file size
> > + *
> > + * This function retrieves the size of the file on the filesystem. The File
> > + * instance shall be open to retrieve its size. The error() status is not
> > + * modified, error codes are returned directly on failure.
> > + *
> > + * \return The file size in bytes on success, or a negative error code otherwise
> > + */
> > +ssize_t File::size() const
> > +{
> > +	if (!isOpen())
> > +		return -EINVAL;
> > +
> > +	struct stat st;
> > +	int ret = fstat(fd_, &st);
> > +	if (ret < 0)
> > +		return -errno;
> > +
> > +	return st.st_size;
> > +}
> > +
> > +/**
> > + * \brief Map a region of the file in the process memory
> > + * \param[in] offset The region offset within the file
> > + * \param[in] size The region sise
> > + * \param[in] flags The mapping flags
> > + *
> > + * This function maps a region of \a size bytes of the file starting at \a
> > + * offset into the process memory. The File instance shall be open, but may be
> > + * closed after mapping the region. Mappings stay valid when the File is
> > + * closed, and are destroyed automatically when the File is deleted.
> 
> I find this a bit dangerous. Would it not be confusing if we allowed a 
> File to be used to open and map /foo/bar, then close it open and map 
> /foo/baz and both mappings are removed when the single a signle File 
> object is destroyed ?

It could be. An alternative would be to remove setFileName(), but to
support file instance embedded in another object for which the name
can't be provided at construction time, we would need a copy operator,
which would bring a different set of API design issues. Do you think
that would be better ? I've modelled the API after the QFile class, but
given that it's been there for a long time in Qt, it may carry
historical API design mistakes. Or maybe given the same reason, its API
may be the result of carefully made decisions that we don't understand
:-)

> > + *
> > + * If \a size is a negative value, this function maps the region starting at \a
> > + * offset until the end of the file.
> > + *
> > + * The mapping memory protection is controlled by the file open mode, unless \a
> > + * flags contains MapPrivate in which case the region is mapped in read/write
> > + * mode.
> > + *
> > + * The error() status is updated.
> > + *
> > + * \return The mapped memory on success, or an empty span otherwise
> > + */
> > +Span<uint8_t> File::map(off_t offset, ssize_t size, enum File::MapFlag flags)
> > +{
> > +	if (!isOpen()) {
> > +		error_ = -EBADF;
> > +		return {};
> > +	}
> > +
> > +	if (size < 0) {
> > +		size = File::size();
> > +		if (size < 0) {
> > +			error_ = size;
> > +			return {};
> > +		}
> > +
> > +		size -= offset;
> > +	}
> > +
> > +	int mmapFlags = flags & MapPrivate ? MAP_PRIVATE : MAP_SHARED;
> > +
> > +	int prot = 0;
> > +	if (mode_ & ReadOnly)
> > +		prot |= PROT_READ;
> > +	if (mode_ & WriteOnly)
> > +		prot |= PROT_WRITE;
> > +	if (flags & MapPrivate)
> > +		prot |= PROT_WRITE;
> > +
> > +	void *map = mmap(NULL, size, prot, mmapFlags, fd_, offset);
> > +	if (map == MAP_FAILED) {
> > +		error_ = -errno;
> > +		return {};
> > +	}
> > +
> > +	maps_.emplace(map, size);
> > +
> > +	error_ = 0;
> > +	return { static_cast<uint8_t *>(map), static_cast<size_t>(size) };
> > +}
> > +
> > +/**
> > + * \brief Unmap a region mapped with map()
> > + * \param[in] addr The region address
> > + *
> > + * The error() status is updated.
> > + *
> > + * \return True on success, or false if an error occurs
> > + */
> > +bool File::unmap(uint8_t *addr)
> > +{
> > +	auto iter = maps_.find(static_cast<void *>(addr));
> > +	if (iter == maps_.end()) {
> > +		error_ = -ENOENT;
> > +		return false;
> > +	}
> > +
> > +	int ret = munmap(addr, iter->second);
> > +	if (ret < 0) {
> > +		error_ = -errno;
> > +		return false;
> > +	}
> > +
> > +	maps_.erase(iter);
> 
> I think error_ shall be set to 0 here right?

Fixed.

> > +	return true;
> > +}
> > +
> > +/**
> > + * \brief Check if the file specified by \a name exists
> > + * \param[in] name The file name
> > + * \return True if the file exists, false otherwise
> > + */
> > +bool File::exists(const std::string &name)
> > +{
> > +	struct stat st;
> > +	int ret = stat(name.c_str(), &st);
> > +	if (ret < 0)
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/include/file.h b/src/libcamera/include/file.h
> > new file mode 100644
> > index 000000000000..ea6f121cb6c5
> > --- /dev/null
> > +++ b/src/libcamera/include/file.h
> > @@ -0,0 +1,69 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * file.h - File I/O operations
> > + */
> > +#ifndef __LIBCAMERA_FILE_H__
> > +#define __LIBCAMERA_FILE_H__
> > +
> > +#include <map>
> > +#include <string>
> > +#include <sys/types.h>
> > +
> > +#include <libcamera/span.h>
> > +
> > +namespace libcamera {
> > +
> > +class File
> > +{
> > +public:
> > +	enum MapFlag {
> > +		MapNoOption = 0,
> > +		MapPrivate = (1 << 0),
> > +	};
> > +
> > +	enum OpenMode {
> > +		NotOpen = 0,
> > +		ReadOnly = (1 << 0),
> > +		WriteOnly = (1 << 1),
> > +		ReadWrite = ReadOnly | WriteOnly,
> > +	};
> > +
> > +	File(const std::string &name);
> > +	File();
> > +	~File();
> > +
> > +	File(const File &) = delete;
> > +	File &operator=(const File &) = delete;
> > +
> > +	const std::string &fileName() const { return name_; }
> > +	void setFileName(const std::string &name);
> > +	bool exists() const;
> > +
> > +	bool open(OpenMode mode);
> 
> The API seems a bit odd here.
> 
> When opening a file we are required to provide how to open it but not 
> which file to open. Would this class be simplified if the file to open 
> would also be provided as an argument to open instead of either to a 
> constructor or setFileName() ?

I like being able to specify the file name separately, to support
exists() for instance. It also helps supporting use cases where the same
file as the be opened and closed multiple times, with a single File
object that would keep the same name. However, as stated above, I can
consider dropping setFileName(), but I can't tell if the API will be
better.

> > +	bool isOpen() const { return fd_ != -1; }
> > +	OpenMode openMode() const { return mode_; }
> > +	void close();
> > +
> > +	int error() const { return error_; }
> > +	ssize_t size() const;
> > +
> > +	Span<uint8_t> map(off_t offset = 0, ssize_t size = -1,
> > +			  MapFlag flags = MapNoOption);
> > +	bool unmap(uint8_t *addr);
> 
> Would it make sens to pass the Span<uint8_t> returned from map as an 
> argument to unmap? Reading the class definition suggest their is an 
> unbalance between new fancy stuff and plain old data ;-)

That I've toyed with. I decided not to do so in order to avoid giving
the impression we support partial unmapping. The span given to unmap()
would either need to have the exact same size as the one returned by
map(), in which case we would need to add error checking internally, or
its size would be ignored. I don't like the second option. The first
option makes it more complicated for the caller, it could allow us to
implement partial unmapping in the future, but I don't think we'll need
that on the File class (and it would be difficult to design right). Do
you think it's worth it ?

> > +
> > +	static bool exists(const std::string &name);
> > +
> > +private:
> > +	std::string name_;
> > +	int fd_;
> > +	OpenMode mode_;
> > +
> > +	int error_;
> > +	std::map<void *, size_t> maps_;
> > +};
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_FILE_H__ */
> > diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build
> > index 17e2bed93fba..921ed5a063cb 100644
> > --- a/src/libcamera/include/meson.build
> > +++ b/src/libcamera/include/meson.build
> > @@ -8,6 +8,7 @@ libcamera_headers = files([
> >      'device_enumerator_sysfs.h',
> >      'device_enumerator_udev.h',
> >      'event_dispatcher_poll.h',
> > +    'file.h',
> >      'formats.h',
> >      'ipa_context_wrapper.h',
> >      'ipa_manager.h',
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 87fa09cde63d..4f5c41678781 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -14,6 +14,7 @@ libcamera_sources = files([
> >      'event_dispatcher.cpp',
> >      'event_dispatcher_poll.cpp',
> >      'event_notifier.cpp',
> > +    'file.cpp',
> >      'file_descriptor.cpp',
> >      'formats.cpp',
> >      'framebuffer_allocator.cpp',

Hi Laurent,

On 2020-04-08 02:45:49 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> On Tue, Apr 07, 2020 at 10:19:34PM +0200, Niklas Söderlund wrote:
> > On 2020-04-04 04:56:16 +0300, Laurent Pinchart wrote:
> > > The File helper class is a RAII wrapper for a file to manage the file
> > > handle and memory-mapped regions.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  src/libcamera/file.cpp            | 338 ++++++++++++++++++++++++++++++
> > >  src/libcamera/include/file.h      |  69 ++++++
> > >  src/libcamera/include/meson.build |   1 +
> > >  src/libcamera/meson.build         |   1 +
> > >  4 files changed, 409 insertions(+)
> > >  create mode 100644 src/libcamera/file.cpp
> > >  create mode 100644 src/libcamera/include/file.h
> > > 
> > > diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp
> > > new file mode 100644
> > > index 000000000000..23cea4aa3101
> > > --- /dev/null
> > > +++ b/src/libcamera/file.cpp
> > > @@ -0,0 +1,338 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2020, Google Inc.
> > > + *
> > > + * file.cpp - File I/O operations
> > > + */
> > > +
> > > +#include "file.h"
> > > +
> > > +#include <errno.h>
> > > +#include <fcntl.h>
> > > +#include <sys/mman.h>
> > > +#include <sys/stat.h>
> > > +#include <sys/types.h>
> > > +#include <unistd.h>
> > > +
> > > +#include "log.h"
> > > +
> > > +/**
> > > + * \file file.h
> > > + * \brief File I/O operations
> > > + */
> > > +
> > > +namespace libcamera {
> > > +
> > > +LOG_DEFINE_CATEGORY(File);
> > > +
> > > +/**
> > > + * \class File
> > > + * \brief Interface for I/O operations on files
> > > + *
> > > + * The File class provides an interface to perform I/O operations on files. It
> > > + * wraps opening, closing and mapping files in memory, and handles the cleaning
> > > + * of allocated resources.
> > > + *
> > > + * File instances are usually constructed with a file name, but the name can be
> > > + * set later through the setFileName() function. Instances are not automatically
> > > + * opened when constructed, and shall be opened explictly with open().
> > > + *
> > > + * Files can be mapped to the process memory with map(). Mapped regions can be
> > > + * unmapped manually with munmap(), and are automatically unmapped when the File
> > > + * is destroyed.
> > > + */
> > > +
> > > +/**
> > > + * \enum File::MapFlag
> > > + * \brief Flags for the File::map() function
> > > + * \var File::MapNoOption
> > > + * \brief No option (used as default value)
> > > + * \var File::MapPrivate
> > > + * \brief The memory region is mapped as private, changes are not reflected in
> > > + * the file constents
> > > + */
> > > +
> > > +/**
> > > + * \enum File::OpenMode
> > > + * \brief Mode in which a file is opened
> > > + * \var File::NotOpen
> > > + * \brief The file is not open
> > > + * \var File::ReadOnly
> > > + * \brief The file is open for reading
> > > + * \var File::WriteOnly
> > > + * \brief The file is open for writing
> > > + * \var File::ReadWrite
> > > + * \brief The file is open for reading and writing
> > > + */
> > > +
> > > +/**
> > > + * \brief Construct a File to represent the file \a name
> > > + * \param[in] name The file name
> > > + *
> > > + * Upon construction the File object is closed and shall be opened with open()
> > > + * before performing I/O operations.
> > > + */
> > > +File::File(const std::string &name)
> > > +	: name_(name), fd_(-1), mode_(NotOpen), error_(0)
> > > +{
> > > +}
> > > +
> > > +/**
> > > + * \brief Construct a File without an associated name
> > > + *
> > > + * Before being used for any purpose, the file name shall be set with
> > > + * setFileName().
> > > + */
> > > +File::File()
> > > +	: fd_(-1), mode_(NotOpen), error_(0)
> > > +{
> > > +}
> > > +
> > > +/**
> > > + * \brief Destroy a File instance
> > > + *
> > > + * Any memory mapping associated with the File is unmapped, and the File is
> > > + * closed if it is open.
> > > + */
> > > +File::~File()
> > > +{
> > > +	for (const auto &map : maps_)
> > > +		munmap(map.first, map.second);
> > > +
> > > +	close();
> > > +}
> > > +
> > > +/**
> > > + * \fn const std::string &File::fileName() const
> > > + * \brief Retrieve the file name
> > > + * \return The file name
> > > + */
> > > +
> > > +/**
> > > + * \brief Set the name of the file
> > > + * \param[in] name The name of the file
> > > + *
> > > + * The \a name can contain an absolute path, a relative path or no path at all.
> > > + * Calling this function on an open file results in undefined behaviour.
> > > + */
> > > +void File::setFileName(const std::string &name)
> > > +{
> > > +	if (isOpen()) {
> > > +		LOG(File, Error)
> > > +			<< "Can't set file name on already open file " << name_;
> > > +		return;
> > > +	}
> > > +
> > > +	name_ = name;
> > > +}
> > > +
> > > +/**
> > > + * \brief Check if the file specified by fileName() exists
> > > + *
> > > + * This function checks if the file specified by fileName() exists. The File
> > > + * instance doesn't need to be open to check for file existence, and this
> > > + * function may return false even if the file is open, if it was deleted from
> > > + * the file system.
> > > + *
> > > + * \return True if the the file exists, false otherwise
> > > + */
> > > +bool File::exists() const
> > > +{
> > > +	return exists(name_);
> > > +}
> > > +
> > > +/**
> > > + * \brief Open the file in the given mode
> > > + * \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.
> > 
> > Shall the updated error status be mentioned here ?
> 
> Fixed.
> 
> > > + *
> > > + * \return True on success, false otherwise
> > > + */
> > > +bool File::open(File::OpenMode mode)
> > > +{
> > > +	if (isOpen()) {
> > > +		LOG(File, Error) << "File " << name_ << " is already open";
> > > +		return false;
> > > +	}
> > > +
> > > +	int flags = (mode & ReadWrite) - 1;
> > > +
> > > +	fd_ = ::open(name_.c_str(), flags);
> > > +	if (fd_ < 0) {
> > > +		error_ = -errno;
> > > +		return false;
> > > +	}
> > > +
> > > +	mode_ = mode;
> > > +	error_ = 0;
> > > +	return true;
> > > +}
> > > +
> > > +/**
> > > + * \fn bool File::isOpen() const
> > > + * \brief Check if the file is open
> > > + * \return True if the file is open, false otherwise
> > > + */
> > > +
> > > +/**
> > > + * \fn OpenMode File::openMode() const
> > > + * \brief Retrieve the file open mode
> > > + * \return The file open mode
> > > + */
> > > +
> > > +/**
> > > + * \brief Close the file
> > > + *
> > > + * This function closes the File. If the File is not open, it performs no
> > > + * operation. Memory mappings created with map() are not destroyed when the
> > > + * file is closed.
> > > + */
> > > +void File::close()
> > > +{
> > > +	if (fd_ == -1)
> > > +		return;
> > > +
> > > +	::close(fd_);
> > > +	fd_ = -1;
> > > +	mode_ = NotOpen;
> > > +}
> > > +
> > > +/**
> > > + * \fn int File::error() const
> > > + * \brief Retrieve the file error status
> > > + *
> > > + * This function retrieves the error status from the last file open or I/O
> > > + * operation. The error status is a negative number as defined by errno.h. If
> > > + * no error occurred, this function returns 0.
> > > + *
> > > + * \return The file error status
> > > + */
> > > +
> > > +/**
> > > + * \brief Retrieve the file size
> > > + *
> > > + * This function retrieves the size of the file on the filesystem. The File
> > > + * instance shall be open to retrieve its size. The error() status is not
> > > + * modified, error codes are returned directly on failure.
> > > + *
> > > + * \return The file size in bytes on success, or a negative error code otherwise
> > > + */
> > > +ssize_t File::size() const
> > > +{
> > > +	if (!isOpen())
> > > +		return -EINVAL;
> > > +
> > > +	struct stat st;
> > > +	int ret = fstat(fd_, &st);
> > > +	if (ret < 0)
> > > +		return -errno;
> > > +
> > > +	return st.st_size;
> > > +}
> > > +
> > > +/**
> > > + * \brief Map a region of the file in the process memory
> > > + * \param[in] offset The region offset within the file
> > > + * \param[in] size The region sise
> > > + * \param[in] flags The mapping flags
> > > + *
> > > + * This function maps a region of \a size bytes of the file starting at \a
> > > + * offset into the process memory. The File instance shall be open, but may be
> > > + * closed after mapping the region. Mappings stay valid when the File is
> > > + * closed, and are destroyed automatically when the File is deleted.
> > 
> > I find this a bit dangerous. Would it not be confusing if we allowed a 
> > File to be used to open and map /foo/bar, then close it open and map 
> > /foo/baz and both mappings are removed when the single a signle File 
> > object is destroyed ?
> 
> It could be. An alternative would be to remove setFileName(), but to
> support file instance embedded in another object for which the name
> can't be provided at construction time, we would need a copy operator,
> which would bring a different set of API design issues. Do you think
> that would be better ? I've modelled the API after the QFile class, but
> given that it's been there for a long time in Qt, it may carry
> historical API design mistakes. Or maybe given the same reason, its API
> may be the result of carefully made decisions that we don't understand
> :-)

It's a good point, that embedding it would be more complicated without 
setFileName(). One option is to provide the name to open(), but as you 
describe it would prevent exists() to be truly useful. You do provide a 
static version of exists() to so the functionality is the same but not 
as nice I agree.

How about a middle ground which address my real concern, disallow 
setFileName() if we have active mappings ? Maybe I'm being to paranoid.

> 
> > > + *
> > > + * If \a size is a negative value, this function maps the region starting at \a
> > > + * offset until the end of the file.
> > > + *
> > > + * The mapping memory protection is controlled by the file open mode, unless \a
> > > + * flags contains MapPrivate in which case the region is mapped in read/write
> > > + * mode.
> > > + *
> > > + * The error() status is updated.
> > > + *
> > > + * \return The mapped memory on success, or an empty span otherwise
> > > + */
> > > +Span<uint8_t> File::map(off_t offset, ssize_t size, enum File::MapFlag flags)
> > > +{
> > > +	if (!isOpen()) {
> > > +		error_ = -EBADF;
> > > +		return {};
> > > +	}
> > > +
> > > +	if (size < 0) {
> > > +		size = File::size();
> > > +		if (size < 0) {
> > > +			error_ = size;
> > > +			return {};
> > > +		}
> > > +
> > > +		size -= offset;
> > > +	}
> > > +
> > > +	int mmapFlags = flags & MapPrivate ? MAP_PRIVATE : MAP_SHARED;
> > > +
> > > +	int prot = 0;
> > > +	if (mode_ & ReadOnly)
> > > +		prot |= PROT_READ;
> > > +	if (mode_ & WriteOnly)
> > > +		prot |= PROT_WRITE;
> > > +	if (flags & MapPrivate)
> > > +		prot |= PROT_WRITE;
> > > +
> > > +	void *map = mmap(NULL, size, prot, mmapFlags, fd_, offset);
> > > +	if (map == MAP_FAILED) {
> > > +		error_ = -errno;
> > > +		return {};
> > > +	}
> > > +
> > > +	maps_.emplace(map, size);
> > > +
> > > +	error_ = 0;
> > > +	return { static_cast<uint8_t *>(map), static_cast<size_t>(size) };
> > > +}
> > > +
> > > +/**
> > > + * \brief Unmap a region mapped with map()
> > > + * \param[in] addr The region address
> > > + *
> > > + * The error() status is updated.
> > > + *
> > > + * \return True on success, or false if an error occurs
> > > + */
> > > +bool File::unmap(uint8_t *addr)
> > > +{
> > > +	auto iter = maps_.find(static_cast<void *>(addr));
> > > +	if (iter == maps_.end()) {
> > > +		error_ = -ENOENT;
> > > +		return false;
> > > +	}
> > > +
> > > +	int ret = munmap(addr, iter->second);
> > > +	if (ret < 0) {
> > > +		error_ = -errno;
> > > +		return false;
> > > +	}
> > > +
> > > +	maps_.erase(iter);
> > 
> > I think error_ shall be set to 0 here right?
> 
> Fixed.
> 
> > > +	return true;
> > > +}
> > > +
> > > +/**
> > > + * \brief Check if the file specified by \a name exists
> > > + * \param[in] name The file name
> > > + * \return True if the file exists, false otherwise
> > > + */
> > > +bool File::exists(const std::string &name)
> > > +{
> > > +	struct stat st;
> > > +	int ret = stat(name.c_str(), &st);
> > > +	if (ret < 0)
> > > +		return false;
> > > +
> > > +	return true;
> > > +}
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/src/libcamera/include/file.h b/src/libcamera/include/file.h
> > > new file mode 100644
> > > index 000000000000..ea6f121cb6c5
> > > --- /dev/null
> > > +++ b/src/libcamera/include/file.h
> > > @@ -0,0 +1,69 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2020, Google Inc.
> > > + *
> > > + * file.h - File I/O operations
> > > + */
> > > +#ifndef __LIBCAMERA_FILE_H__
> > > +#define __LIBCAMERA_FILE_H__
> > > +
> > > +#include <map>
> > > +#include <string>
> > > +#include <sys/types.h>
> > > +
> > > +#include <libcamera/span.h>
> > > +
> > > +namespace libcamera {
> > > +
> > > +class File
> > > +{
> > > +public:
> > > +	enum MapFlag {
> > > +		MapNoOption = 0,
> > > +		MapPrivate = (1 << 0),
> > > +	};
> > > +
> > > +	enum OpenMode {
> > > +		NotOpen = 0,
> > > +		ReadOnly = (1 << 0),
> > > +		WriteOnly = (1 << 1),
> > > +		ReadWrite = ReadOnly | WriteOnly,
> > > +	};
> > > +
> > > +	File(const std::string &name);
> > > +	File();
> > > +	~File();
> > > +
> > > +	File(const File &) = delete;
> > > +	File &operator=(const File &) = delete;
> > > +
> > > +	const std::string &fileName() const { return name_; }
> > > +	void setFileName(const std::string &name);
> > > +	bool exists() const;
> > > +
> > > +	bool open(OpenMode mode);
> > 
> > The API seems a bit odd here.
> > 
> > When opening a file we are required to provide how to open it but not 
> > which file to open. Would this class be simplified if the file to open 
> > would also be provided as an argument to open instead of either to a 
> > constructor or setFileName() ?
> 
> I like being able to specify the file name separately, to support
> exists() for instance. It also helps supporting use cases where the same
> file as the be opened and closed multiple times, with a single File
> object that would keep the same name. However, as stated above, I can
> consider dropping setFileName(), but I can't tell if the API will be
> better.

Would it make sens to trun setFileName() into setFile() and extending it 
and the constructor to take a name and a mode? I feel it's odd that mode 
is specified here more then the file is not.

No strong feeling about. It just feels odd I know which file I want but 
not what I want do do with. To me it feels they go together.

> 
> > > +	bool isOpen() const { return fd_ != -1; }
> > > +	OpenMode openMode() const { return mode_; }
> > > +	void close();
> > > +
> > > +	int error() const { return error_; }
> > > +	ssize_t size() const;
> > > +
> > > +	Span<uint8_t> map(off_t offset = 0, ssize_t size = -1,
> > > +			  MapFlag flags = MapNoOption);
> > > +	bool unmap(uint8_t *addr);
> > 
> > Would it make sens to pass the Span<uint8_t> returned from map as an 
> > argument to unmap? Reading the class definition suggest their is an 
> > unbalance between new fancy stuff and plain old data ;-)
> 
> That I've toyed with. I decided not to do so in order to avoid giving
> the impression we support partial unmapping. The span given to unmap()
> would either need to have the exact same size as the one returned by
> map(), in which case we would need to add error checking internally, or
> its size would be ignored. I don't like the second option. The first
> option makes it more complicated for the caller, it could allow us to
> implement partial unmapping in the future, but I don't think we'll need
> that on the File class (and it would be difficult to design right). Do
> you think it's worth it ?

I don't think we need to support partial unmapping.

I do think taking the Span<> here would make more sens then a uint8_t *.  
I think the error check to make sure the size provided and the one from 
the mapping match would be trivial to add as we already lookup the size 
of the mapping to give it to unmap. I agree ignoring the size if 
provided feels bad.

I reacted to this because I had to look at the test case to know where 
the pointer really was 'expected' to come from.

> 
> > > +
> > > +	static bool exists(const std::string &name);
> > > +
> > > +private:
> > > +	std::string name_;
> > > +	int fd_;
> > > +	OpenMode mode_;
> > > +
> > > +	int error_;
> > > +	std::map<void *, size_t> maps_;
> > > +};
> > > +
> > > +} /* namespace libcamera */
> > > +
> > > +#endif /* __LIBCAMERA_FILE_H__ */
> > > diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build
> > > index 17e2bed93fba..921ed5a063cb 100644
> > > --- a/src/libcamera/include/meson.build
> > > +++ b/src/libcamera/include/meson.build
> > > @@ -8,6 +8,7 @@ libcamera_headers = files([
> > >      'device_enumerator_sysfs.h',
> > >      'device_enumerator_udev.h',
> > >      'event_dispatcher_poll.h',
> > > +    'file.h',
> > >      'formats.h',
> > >      'ipa_context_wrapper.h',
> > >      'ipa_manager.h',
> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > > index 87fa09cde63d..4f5c41678781 100644
> > > --- a/src/libcamera/meson.build
> > > +++ b/src/libcamera/meson.build
> > > @@ -14,6 +14,7 @@ libcamera_sources = files([
> > >      'event_dispatcher.cpp',
> > >      'event_dispatcher_poll.cpp',
> > >      'event_notifier.cpp',
> > > +    'file.cpp',
> > >      'file_descriptor.cpp',
> > >      'formats.cpp',
> > >      'framebuffer_allocator.cpp',
> 
> -- 
> Regards,
> 
> Laurent Pinchart