[libcamera-devel] [PATCH v1 2/5] v4l2: v4l2_camera_file: Store file description

[Kieran Bingham]

Quoting Laurent Pinchart (2021-12-28 21:59:48)
> Create a string that describe the file from the path and file
> descriptor. This will be used in log messages to clearly identify which
> file an operation is related to.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/v4l2/v4l2_camera_file.cpp    | 28 +++++++++++++++++++++++++++-
>  src/v4l2/v4l2_camera_file.h      |  8 +++++++-
>  src/v4l2/v4l2_compat_manager.cpp |  4 +++-
>  3 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera_file.cpp b/src/v4l2/v4l2_camera_file.cpp
> index a07679b587ce..0a41587ca90b 100644
> --- a/src/v4l2/v4l2_camera_file.cpp
> +++ b/src/v4l2/v4l2_camera_file.cpp
> @@ -7,20 +7,46 @@
>  
>  #include "v4l2_camera_file.h"
>  
> +#include <fcntl.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
>  #include <linux/videodev2.h>
>  
>  #include "v4l2_camera_proxy.h"
>  
>  using namespace libcamera;
>  
> -V4L2CameraFile::V4L2CameraFile(int efd, bool nonBlocking, V4L2CameraProxy *proxy)
> +V4L2CameraFile::V4L2CameraFile(int dirfd, const char *path, int efd,
> +                              bool nonBlocking, V4L2CameraProxy *proxy)
>         : proxy_(proxy), nonBlocking_(nonBlocking), efd_(efd),
>           priority_(V4L2_PRIORITY_DEFAULT)
>  {
>         proxy_->open(this);
> +
> +       if (path[0] != '/') {
> +               if (dirfd == AT_FDCWD) {
> +                       char *cwd = getcwd(nullptr, 0);
> +                       if (cwd) {
> +                               description_ = std::string(cwd) + "/";
> +                               free(cwd);
> +                       } else {
> +                               description_ = std::string("(unreachable)/");

I would have thought this could have been './' but I don't expect it to
fail in these circumstances. Though saying that, man getcwd() does state
that the nullptr,0 parameters are a glibc specific extension - so we
could end up with this failing in fact.

So (unreachable)/ is probably fine, but we could also at that point
allocate our own memory to actaully obtain it... (or .. use '.').

Anyway, I don't think that's too important here, as it's probably an
edge case (until someone tests otherwise).

So,


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> +                       }
> +               } else {
> +                       description_ = "(dirfd:" + std::to_string(dirfd) + ")/";
> +               }
> +       }
> +
> +       description_ += std::string(path) + " (fd:" + std::to_string(efd) + ")";
>  }
>  
>  V4L2CameraFile::~V4L2CameraFile()
>  {
>         proxy_->close(this);
>  }
> +
> +const std::string &V4L2CameraFile::description() const
> +{
> +       return description_;
> +}
> diff --git a/src/v4l2/v4l2_camera_file.h b/src/v4l2/v4l2_camera_file.h
> index 6c4cb5d89dbf..1a7b6a63ae95 100644
> --- a/src/v4l2/v4l2_camera_file.h
> +++ b/src/v4l2/v4l2_camera_file.h
> @@ -7,6 +7,8 @@
>  
>  #pragma once
>  
> +#include <string>
> +
>  #include <linux/videodev2.h>
>  
>  class V4L2CameraProxy;
> @@ -14,7 +16,8 @@ class V4L2CameraProxy;
>  class V4L2CameraFile
>  {
>  public:
> -       V4L2CameraFile(int efd, bool nonBlocking, V4L2CameraProxy *proxy);
> +       V4L2CameraFile(int dirfd, const char *path, int efd, bool nonBlocking,
> +                      V4L2CameraProxy *proxy);
>         ~V4L2CameraFile();
>  
>         V4L2CameraProxy *proxy() const { return proxy_; }
> @@ -25,9 +28,12 @@ public:
>         enum v4l2_priority priority() const { return priority_; }
>         void setPriority(enum v4l2_priority priority) { priority_ = priority; }
>  
> +       const std::string &description() const;
> +
>  private:
>         V4L2CameraProxy *proxy_;
>  
> +       std::string description_;
>         bool nonBlocking_;
>         int efd_;
>         enum v4l2_priority priority_;
> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp
> index c64cdb862489..585046e97e4b 100644
> --- a/src/v4l2/v4l2_compat_manager.cpp
> +++ b/src/v4l2/v4l2_compat_manager.cpp
> @@ -156,7 +156,9 @@ int V4L2CompatManager::openat(int dirfd, const char *path, int oflag, mode_t mod
>                 return efd;
>  
>         V4L2CameraProxy *proxy = proxies_[ret].get();
> -       files_.emplace(efd, std::make_shared<V4L2CameraFile>(efd, oflag & O_NONBLOCK, proxy));
> +       files_.emplace(efd, std::make_shared<V4L2CameraFile>(dirfd, path, efd,
> +                                                            oflag & O_NONBLOCK,
> +                                                            proxy));
>  
>         LOG(V4L2Compat, Debug) << "Opened " << path << " -> fd " << efd;
>         return efd;
> -- 
> Regards,
> 
> Laurent Pinchart
>