Message ID | 20211228215951.32396-3-laurent.pinchart@ideasonboard.com |
---|---|
State | Accepted |
Delegated to: | Laurent Pinchart |
Headers | show |
Series |
|
Related | show |
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@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@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 >
Hi Kieran, On Mon, Jan 10, 2022 at 04:41:08PM +0000, Kieran Bingham wrote: > 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@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. I've checked uclibc, musl and bionic, and they all implement the extension, so I think we're good. > So (unreachable)/ is probably fine, but we could also at that point > allocate our own memory to actaully obtain it... (or .. use '.'). The "(unreachable)" name, by the way, comes from Linux 2.6.36 that uses that prefix when the current directory is not below the root directory of the current process. In any case, I don't ever expect users to see this. > 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@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;
Hi Laurent, On Tue, Dec 28, 2021 at 11:59:48PM +0200, Laurent Pinchart wrote: > 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@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@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)/"); > + } > + } 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 >
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)/"); + } + } 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;
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@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(-)