[libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write support
Niklas Söderlund
niklas.soderlund at ragnatech.se
Tue Jul 14 07:52:58 CEST 2020
Hi Laurent,
On 2020-07-14 01:55:36 +0300, Laurent Pinchart wrote:
> Hi Niklas, Kieran,
>
> On Mon, Jul 13, 2020 at 09:31:42AM +0100, Kieran Bingham wrote:
> > Hi Niklas / Laurent,
> >
> > On 13/07/2020 07:53, Niklas Söderlund wrote:
> > > Hi Laurent,
> > >
> > > Thanks for your work.
> > >
> > > On 2020-07-12 17:44:16 +0300, Laurent Pinchart wrote:
> > >> Add basic support to read and write data from/to a file, along with
> > >> retrieving and setting the current read/write position.
> > >>
> > >> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > >> ---
> > >> include/libcamera/internal/file.h | 7 ++
> > >> src/libcamera/file.cpp | 111 +++++++++++++++++++++++++++++-
> > >> 2 files changed, 116 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/include/libcamera/internal/file.h b/include/libcamera/internal/file.h
> > >> index e3e72132e336..ead94cdf29c9 100644
> > >> --- a/include/libcamera/internal/file.h
> > >> +++ b/include/libcamera/internal/file.h
> > >> @@ -49,6 +49,12 @@ public:
> > >> int error() const { return error_; }
> > >> ssize_t size() const;
> > >>
> > >> + off_t pos() const { return pos_; }
> > >
> > > Would it not be nicer to return lseek(fd_, 0, SEEK_CUR) here? That way
> > > we could avoid keeping pos_ in sync in potential corner cases.
> >
> > Aha ... this would of course address my concerns about pos_ becoming
> > invalid on error cases ...
>
> Wouldn't it be more efficient to update pos_ in case of a read or write
> error ? Efficiency may not be our primary concern here though. I don't
> mind much either way, does anyone have a preference ?
No strong preference, but if the kernel keeps track of things for you it
seems folly to reimplement it in user-space :-)
Whatever counters the kernel keep to make SEEK_CUR work it will still do
disregarding if we use it or not. So the only efficiency gain would be
if we read it often and modified it a seldom, in that case I rather just
cache SEEK_CUR and refresh it each time we to a write() or read().
>
> > >> + off_t seek(off_t pos);
> > >> +
> > >> + ssize_t read(const Span<uint8_t> &data);
> > >> + ssize_t write(const Span<const uint8_t> &data);
> > >> +
> > >> Span<uint8_t> map(off_t offset = 0, ssize_t size = -1,
> > >> MapFlag flags = MapNoOption);
> > >> bool unmap(uint8_t *addr);
> > >> @@ -61,6 +67,7 @@ private:
> > >> std::string name_;
> > >> int fd_;
> > >> OpenMode mode_;
> > >> + off_t pos_;
> > >>
> > >> int error_;
> > >> std::map<void *, size_t> maps_;
> > >> diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp
> > >> index c471bde3fc68..8bd109090919 100644
> > >> --- a/src/libcamera/file.cpp
> > >> +++ b/src/libcamera/file.cpp
> > >> @@ -73,7 +73,7 @@ LOG_DEFINE_CATEGORY(File);
> > >> * before performing I/O operations.
> > >> */
> > >> File::File(const std::string &name)
> > >> - : name_(name), fd_(-1), mode_(NotOpen), error_(0)
> > >> + : name_(name), fd_(-1), mode_(NotOpen), pos_(0), error_(0)
> > >> {
> > >> }
> > >>
> > >> @@ -84,7 +84,7 @@ File::File(const std::string &name)
> > >> * setFileName().
> > >> */
> > >> File::File()
> > >> - : fd_(-1), mode_(NotOpen), error_(0)
> > >> + : fd_(-1), mode_(NotOpen), pos_(0), error_(0)
> > >> {
> > >> }
> > >>
> > >> @@ -202,6 +202,7 @@ void File::close()
> > >> ::close(fd_);
> > >> fd_ = -1;
> > >> mode_ = NotOpen;
> > >> + pos_ = 0;
> > >> }
> > >>
> > >> /**
> > >> @@ -237,6 +238,112 @@ ssize_t File::size() const
> > >> return st.st_size;
> > >> }
> > >>
> > >> +/**
> > >> + * \fn off_t File::pos() const
> > >> + * \brief Return current read or write position
> > >> + *
> > >> + * If the file is closed, this function returns 0.
> > >> + *
> > >> + * \return The current read or write position
> > >> + */
> > >> +
> > >> +/**
> > >> + * \brief Set the read or write position
> > >> + * \param[in] pos The desired position
> > >> + * \return The resulting offset from the beginning of the file on success, or a
> > >> + * negative error code otherwise
> > >> + */
> > >> +off_t File::seek(off_t pos)
> > >> +{
> > >> + if (!isOpen())
> > >> + return -EINVAL;
> > >> +
> > >> + off_t ret = lseek(fd_, pos, SEEK_SET);
> > >> + if (ret < 0)
> > >> + return -errno;
> > >> +
> > >> + pos_ = ret;
> > >> + return ret;
> > >> +}
> > >> +
> > >> +/**
> > >> + * \brief Read data from the file
> > >> + * \param[in] data Memory to read data into
> > >> + *
> > >> + * Read at most \a data.size() bytes from the file into \a data.data(), and
> > >> + * return the number of bytes read. If less data than requested is available,
> > >> + * the returned byte count may be smaller than the requested size. If no more
> > >> + * data is available, 0 is returned.
> > >> + *
> > >> + * The position of the file as returned by pos() is advanced by the number of
> > >> + * bytes read. If an error occurs, the position isn't modified.
> > >> + *
> > >> + * \return The number of bytes read on success, or a negative error code
> > >> + * otherwise
> > >> + */
> > >> +ssize_t File::read(const Span<uint8_t> &data)
> > >> +{
> > >> + if (!isOpen())
> > >> + return -EINVAL;
> > >> +
> > >> + size_t readBytes = 0;
> > >> + ssize_t ret = 0;
> > >> +
> > >> + /* Retry in case of interrupted system calls. */
> > >> + while (readBytes < data.size()) {
> > >> + ret = ::read(fd_, data.data() + readBytes,
> > >> + data.size() - readBytes);
> > >> + if (ret <= 0)
> > >> + break;
> > >> +
> > >> + readBytes += ret;
> > >> + }
> > >> +
> > >> + if (ret < 0 && !readBytes)
> > >> + return -errno;
> > >> +
> > >> + pos_ += readBytes;
> > >> + return readBytes;
> > >> +}
> > >> +
> > >> +/**
> > >> + * \brief Write data to the file
> > >> + * \param[in] data Memory containing data to be written
> > >> + *
> > >> + * Write at most \a data.size() bytes from \a data.data() to the file, and
> > >> + * return the number of bytes written. If the file system doesn't have enough
> > >> + * space for the data, the returned byte count may be less than requested.
> > >> + *
> > >> + * The position of the file as returned by pos() is advanced by the number of
> > >> + * bytes written. If an error occurs, the position isn't modified.
> > >> + *
> > >> + * \return The number of bytes written on success, or a negative error code
> > >> + * otherwise
> > >> + */
> > >> +ssize_t File::write(const Span<const uint8_t> &data)
> > >> +{
> > >> + if (!isOpen())
> > >> + return -EINVAL;
> > >> +
> > >> + size_t writtenBytes = 0;
> > >> +
> > >> + /* Retry in case of interrupted system calls. */
> > >> + while (writtenBytes < data.size()) {
> > >> + ssize_t ret = ::write(fd_, data.data() + writtenBytes,
> > >> + data.size() - writtenBytes);
> > >> + if (ret <= 0)
> > >> + break;
> > >> +
> > >> + writtenBytes += ret;
> > >> + }
> > >> +
> > >> + if (data.size() && !writtenBytes)
> > >> + return -errno;
> > >> +
> > >> + pos_ += writtenBytes;
> > >> + return writtenBytes;
> > >> +}
> > >> +
> > >> /**
> > >> * \brief Map a region of the file in the process memory
> > >> * \param[in] offset The region offset within the file
>
> --
> Regards,
>
> Laurent Pinchart
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list