1
0
Fork 0

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>
This commit is contained in:
Roman Stratiienko 2021-09-29 12:46:28 +03:00
parent 0a90b27c81
commit 3e8ce57ada
8 changed files with 118 additions and 52 deletions

View file

@ -4,6 +4,7 @@
TIDY_FILES=(
drm/DrmFbImporter.h
drm/DrmUnique.h
utils/UniqueFd.h
utils/log.h
utils/properties.h

View file

@ -962,9 +962,9 @@ HWC2::Error DrmHwcTwo::HwcDisplay::GetDisplayIdentificationData(
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;
}

View file

@ -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 @@ DrmDisplayCompositor::GetActiveModeResolution() {
}
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 @@ int DrmDisplayCompositor::DisablePlanes(DrmDisplayComposition *display_comp) {
->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 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
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 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
(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 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
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 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
crtc->id()) < 0;
if (ret) {
ALOGE("Failed to add blob %d to pset", mode_.blob_id);
drmModeAtomicFree(pset);
return ret;
}
}
@ -511,12 +507,9 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
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);

View file

@ -90,20 +90,19 @@ int DrmConnector::UpdateEdidProperty() {
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 {

View file

@ -26,6 +26,7 @@
#include "DrmEncoder.h"
#include "DrmMode.h"
#include "DrmProperty.h"
#include "DrmUnique.h"
namespace android {
@ -41,7 +42,7 @@ class DrmConnector {
int Init();
int UpdateEdidProperty();
int GetEdidBlob(drmModePropertyBlobPtr &blob);
auto GetEdidBlob() -> DrmModePropertyBlobUnique;
uint32_t id() const;

View file

@ -161,7 +161,7 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
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 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
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::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
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::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
}
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 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
}
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::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
}
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 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
}
}
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 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
planes_.emplace_back(std::move(plane));
}
drmModeFreePlaneResources(plane_res);
if (ret)
return std::make_tuple(ret, 0);

85
drm/DrmUnique.h Normal file
View file

@ -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

View file

@ -103,22 +103,19 @@ DrmConnector *ResourceManager::AvailableWritebackConnector(int display) {
}
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;
}