[libcamera-devel] [RFC PATCH 06/10] libcamera: V4L2Device: Use ScopedFD for a file descriptor

[Hirokazu Honda]

V4L2Device owns a file descriptor for a v4l2 device node. It
should be managed by ScopedFD avoid the leakage.

Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
---
 include/libcamera/internal/v4l2_device.h |  9 +++++----
 src/libcamera/v4l2_device.cpp            | 17 +++++++----------
 src/libcamera/v4l2_videodevice.cpp       |  3 ++-
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
index d006bf68..e0262de0 100644
--- a/include/libcamera/internal/v4l2_device.h
+++ b/include/libcamera/internal/v4l2_device.h
@@ -13,6 +13,7 @@
 
 #include <linux/videodev2.h>
 
+#include <libcamera/scoped_file_descriptor.h>
 #include <libcamera/signal.h>
 
 #include "libcamera/internal/log.h"
@@ -26,7 +27,7 @@ class V4L2Device : protected Loggable
 {
 public:
 	void close();
-	bool isOpen() const { return fd_ != -1; }
+	bool isOpen() const { return fd_.isValid(); }
 
 	const ControlInfoMap &controls() const { return controls_; }
 
@@ -46,11 +47,11 @@ protected:
 	~V4L2Device();
 
 	int open(unsigned int flags);
-	int setFd(int fd);
+	int setFd(ScopedFD fd);
 
 	int ioctl(unsigned long request, void *argp);
 
-	int fd() const { return fd_; }
+	int fd() const { return fd_.get(); }
 
 private:
 	void listControls();
@@ -64,7 +65,7 @@ private:
 	std::vector<std::unique_ptr<V4L2ControlId>> controlIds_;
 	ControlInfoMap controls_;
 	std::string deviceNode_;
-	int fd_;
+	ScopedFD fd_;
 
 	EventNotifier *fdEventNotifier_;
 	bool frameStartEnabled_;
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
index decd19ef..4fbb2d60 100644
--- a/src/libcamera/v4l2_device.cpp
+++ b/src/libcamera/v4l2_device.cpp
@@ -53,7 +53,7 @@ LOG_DEFINE_CATEGORY(V4L2)
  * at open() time, and the \a logTag to prefix log messages with.
  */
 V4L2Device::V4L2Device(const std::string &deviceNode)
-	: deviceNode_(deviceNode), fd_(-1), fdEventNotifier_(nullptr),
+	: deviceNode_(deviceNode), fdEventNotifier_(nullptr),
 	  frameStartEnabled_(false)
 {
 }
@@ -89,7 +89,7 @@ int V4L2Device::open(unsigned int flags)
 		return ret;
 	}
 
-	setFd(ret);
+	setFd(ScopedFD(ret));
 
 	listControls();
 
@@ -112,14 +112,14 @@ int V4L2Device::open(unsigned int flags)
  *
  * \return 0 on success or a negative error code otherwise
  */
-int V4L2Device::setFd(int fd)
+int V4L2Device::setFd(ScopedFD fd)
 {
 	if (isOpen())
 		return -EBUSY;
 
-	fd_ = fd;
+	fd_ = std::move(fd);
 
-	fdEventNotifier_ = new EventNotifier(fd_, EventNotifier::Exception);
+	fdEventNotifier_ = new EventNotifier(fd_.get(), EventNotifier::Exception);
 	fdEventNotifier_->activated.connect(this, &V4L2Device::eventAvailable);
 	fdEventNotifier_->setEnabled(false);
 
@@ -138,10 +138,7 @@ void V4L2Device::close()
 
 	delete fdEventNotifier_;
 
-	if (::close(fd_) < 0)
-		LOG(V4L2, Error) << "Failed to close V4L2 device: "
-				 << strerror(errno);
-	fd_ = -1;
+	fd_.reset();
 }
 
 /**
@@ -449,7 +446,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
 	 * Printing out an error message is usually better performed
 	 * in the caller, which can provide more context.
 	 */
-	if (::ioctl(fd_, request, argp) < 0)
+	if (::ioctl(fd_.get(), request, argp) < 0)
 		return -errno;
 
 	return 0;
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index 12c09dc7..0bf3b5f5 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -22,6 +22,7 @@
 #include <linux/version.h>
 
 #include <libcamera/file_descriptor.h>
+#include <libcamera/scoped_file_descriptor.h>
 
 #include "libcamera/internal/event_notifier.h"
 #include "libcamera/internal/log.h"
@@ -614,7 +615,7 @@ int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type)
 		return ret;
 	}
 
-	ret = V4L2Device::setFd(newFd);
+	ret = V4L2Device::setFd(ScopedFD(newFd));
 	if (ret < 0) {
 		LOG(V4L2, Error) << "Failed to set file handle: "
 				 << strerror(-ret);
-- 
2.31.1.368.gbe11c130af-goog