drm_hwcomposer: Wrap libdrm drmMode*{Get|Free}* into RAII
This should elliminate chance of any leaks in the future.
Fix drmModePropertyBlobPtr leak in DrmHwcTwo.cpp.
Signed-off-by: Roman Stratiienko <roman.o.stratiienko@globallogic.com>
diff --git a/.ci/.gitlab-ci-clang-tidy-fine.sh b/.ci/.gitlab-ci-clang-tidy-fine.sh
index c1805e8..0e3e35b 100755
--- a/.ci/.gitlab-ci-clang-tidy-fine.sh
+++ b/.ci/.gitlab-ci-clang-tidy-fine.sh
@@ -4,6 +4,7 @@
TIDY_FILES=(
drm/DrmFbImporter.h
+drm/DrmUnique.h
utils/UniqueFd.h
utils/log.h
utils/properties.h
diff --git a/DrmHwcTwo.cpp b/DrmHwcTwo.cpp
index d9ba4f3..968d3ce 100644
--- a/DrmHwcTwo.cpp
+++ b/DrmHwcTwo.cpp
@@ -962,9 +962,9 @@
uint8_t *outPort, uint32_t *outDataSize, uint8_t *outData) {
supported(__func__);
- drmModePropertyBlobPtr blob = nullptr;
+ auto blob = connector_->GetEdidBlob();
- if (connector_->GetEdidBlob(blob)) {
+ if (!blob) {
ALOGE("Failed to get edid property value.");
return HWC2::Error::Unsupported;
}
diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp
index ff9f6ad..142f574 100644
--- a/compositor/DrmDisplayCompositor.cpp
+++ b/compositor/DrmDisplayCompositor.cpp
@@ -34,6 +34,7 @@
#include "drm/DrmCrtc.h"
#include "drm/DrmDevice.h"
#include "drm/DrmPlane.h"
+#include "drm/DrmUnique.h"
#include "utils/autolock.h"
#include "utils/log.h"
@@ -165,7 +166,7 @@
}
int DrmDisplayCompositor::DisablePlanes(DrmDisplayComposition *display_comp) {
- drmModeAtomicReqPtr pset = drmModeAtomicAlloc();
+ auto pset = MakeDrmModeAtomicReqUnique();
if (!pset) {
ALOGE("Failed to allocate property set");
return -ENOMEM;
@@ -176,25 +177,22 @@
->composition_planes();
for (DrmCompositionPlane &comp_plane : comp_planes) {
DrmPlane *plane = comp_plane.plane();
- ret = drmModeAtomicAddProperty(pset, plane->id(),
+ ret = drmModeAtomicAddProperty(pset.get(), plane->id(),
plane->crtc_property().id(), 0) < 0 ||
- drmModeAtomicAddProperty(pset, plane->id(), plane->fb_property().id(),
- 0) < 0;
+ drmModeAtomicAddProperty(pset.get(), plane->id(),
+ plane->fb_property().id(), 0) < 0;
if (ret) {
ALOGE("Failed to add plane %d disable to pset", plane->id());
- drmModeAtomicFree(pset);
return ret;
}
}
DrmDevice *drm = resource_manager_->GetDrmDevice(display_);
- ret = drmModeAtomicCommit(drm->fd(), pset, 0, drm);
+ ret = drmModeAtomicCommit(drm->fd(), pset.get(), 0, drm);
if (ret) {
ALOGE("Failed to commit pset ret=%d\n", ret);
- drmModeAtomicFree(pset);
return ret;
}
- drmModeAtomicFree(pset);
return 0;
}
@@ -221,7 +219,8 @@
return -ENODEV;
}
- drmModeAtomicReqPtr pset = drmModeAtomicAlloc();
+ auto pset_unique = MakeDrmModeAtomicReqUnique();
+ auto *pset = pset_unique.get();
if (!pset) {
ALOGE("Failed to allocate property set");
return -ENOMEM;
@@ -233,7 +232,6 @@
(uint64_t)&out_fences[crtc->pipe()]);
if (ret < 0) {
ALOGE("Failed to add OUT_FENCE_PTR property to pset: %d", ret);
- drmModeAtomicFree(pset);
return ret;
}
}
@@ -243,7 +241,6 @@
crtc->active_property().id(), 1);
if (ret < 0) {
ALOGE("Failed to add crtc active to pset\n");
- drmModeAtomicFree(pset);
return ret;
}
@@ -254,7 +251,6 @@
crtc->id()) < 0;
if (ret) {
ALOGE("Failed to add blob %d to pset", mode_.blob_id);
- drmModeAtomicFree(pset);
return ret;
}
}
@@ -511,12 +507,9 @@
if (ret) {
if (!test_only)
ALOGE("Failed to commit pset ret=%d\n", ret);
- drmModeAtomicFree(pset);
return ret;
}
}
- if (pset)
- drmModeAtomicFree(pset);
if (!test_only && mode_.needs_modeset) {
ret = drm->DestroyPropertyBlob(mode_.old_blob_id);
diff --git a/drm/DrmConnector.cpp b/drm/DrmConnector.cpp
index 0468527..b3179c7 100644
--- a/drm/DrmConnector.cpp
+++ b/drm/DrmConnector.cpp
@@ -90,20 +90,19 @@
return ret;
}
-int DrmConnector::GetEdidBlob(drmModePropertyBlobPtr &blob) {
+auto DrmConnector::GetEdidBlob() -> DrmModePropertyBlobUnique {
uint64_t blob_id = 0;
int ret = UpdateEdidProperty();
- if (ret) {
- return ret;
+ if (ret != 0) {
+ return DrmModePropertyBlobUnique();
}
std::tie(ret, blob_id) = edid_property().value();
- if (ret) {
- return ret;
+ if (ret != 0) {
+ return DrmModePropertyBlobUnique();
}
- blob = drmModeGetPropertyBlob(drm_->fd(), blob_id);
- return !blob;
+ return MakeDrmModePropertyBlobUnique(drm_->fd(), blob_id);
}
uint32_t DrmConnector::id() const {
diff --git a/drm/DrmConnector.h b/drm/DrmConnector.h
index 3bc18c8..e2789ea 100644
--- a/drm/DrmConnector.h
+++ b/drm/DrmConnector.h
@@ -26,6 +26,7 @@
#include "DrmEncoder.h"
#include "DrmMode.h"
#include "DrmProperty.h"
+#include "DrmUnique.h"
namespace android {
@@ -41,7 +42,7 @@
int Init();
int UpdateEdidProperty();
- int GetEdidBlob(drmModePropertyBlobPtr &blob);
+ auto GetEdidBlob() -> DrmModePropertyBlobUnique;
uint32_t id() const;
diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp
index 570b676..bd4b1e4 100644
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -161,7 +161,7 @@
return std::make_tuple(-EACCES, 0);
}
- drmModeResPtr res = drmModeGetResources(fd());
+ auto res = MakeDrmModeResUnique(fd());
if (!res) {
ALOGE("Failed to get DrmDevice resources");
return std::make_tuple(-ENODEV, 0);
@@ -177,15 +177,14 @@
bool found_primary = num_displays != 0;
for (int i = 0; !ret && i < res->count_crtcs; ++i) {
- drmModeCrtcPtr c = drmModeGetCrtc(fd(), res->crtcs[i]);
+ auto c = MakeDrmModeCrtcUnique(fd(), res->crtcs[i]);
if (!c) {
ALOGE("Failed to get crtc %d", res->crtcs[i]);
ret = -ENODEV;
break;
}
- std::unique_ptr<DrmCrtc> crtc(new DrmCrtc(this, c, i));
- drmModeFreeCrtc(c);
+ std::unique_ptr<DrmCrtc> crtc(new DrmCrtc(this, c.get(), i));
ret = crtc->Init();
if (ret) {
@@ -197,7 +196,7 @@
std::vector<uint32_t> possible_clones;
for (int i = 0; !ret && i < res->count_encoders; ++i) {
- drmModeEncoderPtr e = drmModeGetEncoder(fd(), res->encoders[i]);
+ auto e = MakeDrmModeEncoderUnique(fd(), res->encoders[i]);
if (!e) {
ALOGE("Failed to get encoder %d", res->encoders[i]);
ret = -ENODEV;
@@ -215,9 +214,8 @@
}
std::unique_ptr<DrmEncoder> enc(
- new DrmEncoder(e, current_crtc, possible_crtcs));
+ new DrmEncoder(e.get(), current_crtc, possible_crtcs));
possible_clones.push_back(e->possible_clones);
- drmModeFreeEncoder(e);
encoders_.emplace_back(std::move(enc));
}
@@ -229,7 +227,7 @@
}
for (int i = 0; !ret && i < res->count_connectors; ++i) {
- drmModeConnectorPtr c = drmModeGetConnector(fd(), res->connectors[i]);
+ auto c = MakeDrmModeConnectorUnique(fd(), res->connectors[i]);
if (!c) {
ALOGE("Failed to get connector %d", res->connectors[i]);
ret = -ENODEV;
@@ -248,9 +246,7 @@
}
std::unique_ptr<DrmConnector> conn(
- new DrmConnector(this, c, current_encoder, possible_encoders));
-
- drmModeFreeConnector(c);
+ new DrmConnector(this, c.get(), current_encoder, possible_encoders));
ret = conn->Init();
if (ret) {
@@ -299,30 +295,25 @@
}
}
- if (res)
- drmModeFreeResources(res);
-
// Catch-all for the above loops
if (ret)
return std::make_tuple(ret, 0);
- drmModePlaneResPtr plane_res = drmModeGetPlaneResources(fd());
+ auto plane_res = MakeDrmModePlaneResUnique(fd());
if (!plane_res) {
ALOGE("Failed to get plane resources");
return std::make_tuple(-ENOENT, 0);
}
for (uint32_t i = 0; i < plane_res->count_planes; ++i) {
- drmModePlanePtr p = drmModeGetPlane(fd(), plane_res->planes[i]);
+ auto p = MakeDrmModePlaneUnique(fd(), plane_res->planes[i]);
if (!p) {
ALOGE("Failed to get plane %d", plane_res->planes[i]);
ret = -ENODEV;
break;
}
- std::unique_ptr<DrmPlane> plane(new DrmPlane(this, p));
-
- drmModeFreePlane(p);
+ std::unique_ptr<DrmPlane> plane(new DrmPlane(this, p.get()));
ret = plane->Init();
if (ret) {
@@ -332,7 +323,6 @@
planes_.emplace_back(std::move(plane));
}
- drmModeFreePlaneResources(plane_res);
if (ret)
return std::make_tuple(ret, 0);
diff --git a/drm/DrmUnique.h b/drm/DrmUnique.h
new file mode 100644
index 0000000..4d248bf
--- /dev/null
+++ b/drm/DrmUnique.h
@@ -0,0 +1,85 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef DRM_UNIQUE_H_
+#define DRM_UNIQUE_H_
+
+#include <xf86drmMode.h>
+
+#include <functional>
+#include <memory>
+
+template <typename T>
+using DUniquePtr = std::unique_ptr<T, std::function<void(T *)>>;
+
+using DrmModeAtomicReqUnique = DUniquePtr<drmModeAtomicReq>;
+auto inline MakeDrmModeAtomicReqUnique() {
+ return DrmModeAtomicReqUnique(drmModeAtomicAlloc(), [](drmModeAtomicReq *it) {
+ drmModeAtomicFree(it);
+ });
+};
+
+using DrmModeConnectorUnique = DUniquePtr<drmModeConnector>;
+auto inline MakeDrmModeConnectorUnique(int fd, uint32_t connector_id) {
+ return DrmModeConnectorUnique(drmModeGetConnector(fd, connector_id),
+ [](drmModeConnector *it) {
+ drmModeFreeConnector(it);
+ });
+}
+
+using DrmModeCrtcUnique = DUniquePtr<drmModeCrtc>;
+auto inline MakeDrmModeCrtcUnique(int fd, uint32_t crtc_id) {
+ return DrmModeCrtcUnique(drmModeGetCrtc(fd, crtc_id),
+ [](drmModeCrtc *it) { drmModeFreeCrtc(it); });
+}
+
+using DrmModeEncoderUnique = DUniquePtr<drmModeEncoder>;
+auto inline MakeDrmModeEncoderUnique(int fd, uint32_t encoder_id) {
+ return DrmModeEncoderUnique(drmModeGetEncoder(fd, encoder_id),
+ [](drmModeEncoder *it) {
+ drmModeFreeEncoder(it);
+ });
+}
+
+using DrmModePlaneUnique = DUniquePtr<drmModePlane>;
+auto inline MakeDrmModePlaneUnique(int fd, uint32_t plane_id) {
+ return DrmModePlaneUnique(drmModeGetPlane(fd, plane_id),
+ [](drmModePlane *it) { drmModeFreePlane(it); });
+}
+
+using DrmModePlaneResUnique = DUniquePtr<drmModePlaneRes>;
+auto inline MakeDrmModePlaneResUnique(int fd) {
+ return DrmModePlaneResUnique(drmModeGetPlaneResources(fd),
+ [](drmModePlaneRes *it) {
+ drmModeFreePlaneResources(it);
+ });
+}
+
+using DrmModePropertyBlobUnique = DUniquePtr<drmModePropertyBlobRes>;
+auto inline MakeDrmModePropertyBlobUnique(int fd, uint32_t blob_id) {
+ return DrmModePropertyBlobUnique(drmModeGetPropertyBlob(fd, blob_id),
+ [](drmModePropertyBlobRes *it) {
+ drmModeFreePropertyBlob(it);
+ });
+}
+
+using DrmModeResUnique = DUniquePtr<drmModeRes>;
+auto inline MakeDrmModeResUnique(int fd) {
+ return DrmModeResUnique(drmModeGetResources(fd),
+ [](drmModeRes *it) { drmModeFreeResources(it); });
+}
+
+#endif
diff --git a/drm/ResourceManager.cpp b/drm/ResourceManager.cpp
index ef44180..224e709 100644
--- a/drm/ResourceManager.cpp
+++ b/drm/ResourceManager.cpp
@@ -103,22 +103,19 @@
}
bool ResourceManager::IsKMSDev(const char *path) {
- int fd = open(path, O_RDWR | O_CLOEXEC);
- if (fd < 0)
+ auto fd = UniqueFd(open(path, O_RDWR | O_CLOEXEC));
+ if (!fd) {
return false;
+ }
- auto *res = drmModeGetResources(fd);
+ auto res = MakeDrmModeResUnique(fd.Get());
if (!res) {
- close(fd);
return false;
}
bool is_kms = res->count_crtcs > 0 && res->count_connectors > 0 &&
res->count_encoders > 0;
- drmModeFreeResources(res);
- close(fd);
-
return is_kms;
}