[libcamera-devel] [PATCH 2/3] cam: kms_sink: Make lifetime management of DRM request safer

[Laurent Pinchart]

The drmRequest is KMSSink::processRequest() is created as a naked
pointer, passed to the constructor of the KMSSink::Request object that
stores it in a std::unique_ptr<>, and used later in the function. The
current implementation is safe, but could be prone to both memory leaks
and use-after-free bugs if modified. Improve it by replacing the naked
pointer with a std::unique_ptr<>.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 src/cam/kms_sink.cpp | 7 ++++---
 src/cam/kms_sink.h   | 5 +++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp
index 37a3bd50a2bf..16435ede6b6a 100644
--- a/src/cam/kms_sink.cpp
+++ b/src/cam/kms_sink.cpp
@@ -301,7 +301,8 @@ bool KMSSink::processRequest(libcamera::Request *camRequest)
 	DRM::FrameBuffer *drmBuffer = iter->second.get();
 
 	unsigned int flags = DRM::AtomicRequest::FlagAsync;
-	DRM::AtomicRequest *drmRequest = new DRM::AtomicRequest(&dev_);
+	std::unique_ptr<DRM::AtomicRequest> drmRequest =
+		std::make_unique<DRM::AtomicRequest>(&dev_);
 	drmRequest->addProperty(plane_, "FB_ID", drmBuffer->id());
 
 	if (!active_ && !queued_) {
@@ -324,12 +325,12 @@ bool KMSSink::processRequest(libcamera::Request *camRequest)
 		flags |= DRM::AtomicRequest::FlagAllowModeset;
 	}
 
-	pending_ = std::make_unique<Request>(drmRequest, camRequest);
+	pending_ = std::make_unique<Request>(std::move(drmRequest), camRequest);
 
 	std::lock_guard<std::mutex> lock(lock_);
 
 	if (!queued_) {
-		int ret = drmRequest->commit(flags);
+		int ret = pending_->drmRequest_->commit(flags);
 		if (ret < 0) {
 			std::cerr
 				<< "Failed to commit atomic request: "
diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h
index 4a0a872cb653..8f5f08667cea 100644
--- a/src/cam/kms_sink.h
+++ b/src/cam/kms_sink.h
@@ -38,8 +38,9 @@ private:
 	class Request
 	{
 	public:
-		Request(DRM::AtomicRequest *drmRequest, libcamera::Request *camRequest)
-			: drmRequest_(drmRequest), camRequest_(camRequest)
+		Request(std::unique_ptr<DRM::AtomicRequest> drmRequest,
+			libcamera::Request *camRequest)
+			: drmRequest_(std::move(drmRequest)), camRequest_(camRequest)
 		{
 		}
 
-- 
Regards,

Laurent Pinchart