From 8cce3b98cebd6910a9c94c11a6efb2293d2031bc Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Thu, 25 Sep 2025 01:44:33 +0200 Subject: [PATCH] shm: refactor to UP and correct m_data check (#11820) use unique pointers and rvalue references where applicable, buffer is still a shared pointer because CHLBufferReference uses it to hold it locked. in CSHMPool destructor add a check for m_data != MAP_FAILED same in resize, because mmap returns (void *) -1 on failure and that is not comparable to nullptr. delete default constructor so we dont end up in weird states with m_data. --- src/protocols/core/Shm.cpp | 23 ++++++++++++++--------- src/protocols/core/Shm.hpp | 15 ++++++++------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/protocols/core/Shm.cpp b/src/protocols/core/Shm.cpp index 673bc03b..43c087bc 100644 --- a/src/protocols/core/Shm.cpp +++ b/src/protocols/core/Shm.cpp @@ -9,7 +9,10 @@ #include "../../render/Renderer.hpp" using namespace Hyprutils::OS; -CWLSHMBuffer::CWLSHMBuffer(SP pool_, uint32_t id, int32_t offset_, const Vector2D& size_, int32_t stride_, uint32_t fmt_) { +CWLSHMBuffer::CWLSHMBuffer(WP pool_, uint32_t id, int32_t offset_, const Vector2D& size_, int32_t stride_, uint32_t fmt_) { + if UNLIKELY (!pool_) + return; + if UNLIKELY (!pool_->m_pool->m_data) return; @@ -79,14 +82,16 @@ CSHMPool::CSHMPool(CFileDescriptor fd_, size_t size_) : m_fd(std::move(fd_)), m_ } CSHMPool::~CSHMPool() { - munmap(m_data, m_size); + if (m_data != MAP_FAILED) + munmap(m_data, m_size); } void CSHMPool::resize(size_t size_) { LOGM(LOG, "Resizing a SHM pool from {} to {}", m_size, size_); - if (m_data) + if (m_data != MAP_FAILED) munmap(m_data, m_size); + m_size = size_; m_data = mmap(nullptr, m_size, PROT_READ | PROT_WRITE, MAP_SHARED, m_fd.get(), 0); @@ -104,12 +109,12 @@ static int shmIsSizeValid(CFileDescriptor& fd, size_t size) { return sc(st.st_size) >= size; } -CWLSHMPoolResource::CWLSHMPoolResource(SP resource_, CFileDescriptor fd_, size_t size_) : m_resource(resource_) { +CWLSHMPoolResource::CWLSHMPoolResource(UP&& resource_, CFileDescriptor fd_, size_t size_) : m_resource(std::move(resource_)) { if UNLIKELY (!good()) return; if UNLIKELY (!shmIsSizeValid(fd_, size_)) { - resource_->error(-1, "The size of the file is not big enough for the shm pool"); + m_resource->error(-1, "The size of the file is not big enough for the shm pool"); return; } @@ -147,7 +152,7 @@ CWLSHMPoolResource::CWLSHMPoolResource(SP resource_, CFileDescriptor return; } - const auto RESOURCE = PROTO::shm->m_buffers.emplace_back(makeShared(m_self.lock(), id, offset, Vector2D{w, h}, stride, fmt)); + const auto& RESOURCE = PROTO::shm->m_buffers.emplace_back(makeShared(m_self, id, offset, Vector2D{w, h}, stride, fmt)); if UNLIKELY (!RESOURCE->good()) { r->noMemory(); @@ -167,7 +172,7 @@ bool CWLSHMPoolResource::good() { return m_resource->resource(); } -CWLSHMResource::CWLSHMResource(SP resource_) : m_resource(resource_) { +CWLSHMResource::CWLSHMResource(UP&& resource_) : m_resource(std::move(resource_)) { if UNLIKELY (!good()) return; @@ -175,7 +180,7 @@ CWLSHMResource::CWLSHMResource(SP resource_) : m_resource(resource_) { m_resource->setCreatePool([](CWlShm* r, uint32_t id, int32_t fd, int32_t size) { CFileDescriptor poolFd{fd}; - const auto RESOURCE = PROTO::shm->m_pools.emplace_back(makeShared(makeShared(r->client(), r->version(), id), std::move(poolFd), size)); + const auto& RESOURCE = PROTO::shm->m_pools.emplace_back(makeUnique(makeUnique(r->client(), r->version(), id), std::move(poolFd), size)); if UNLIKELY (!RESOURCE->good()) { r->noMemory(); @@ -214,7 +219,7 @@ void CWLSHMProtocol::bindManager(wl_client* client, void* data, uint32_t ver, ui } } - const auto RESOURCE = m_managers.emplace_back(makeShared(makeShared(client, ver, id))); + const auto& RESOURCE = m_managers.emplace_back(makeUnique(makeUnique(client, ver, id))); if UNLIKELY (!RESOURCE->good()) { wl_client_post_no_memory(client); diff --git a/src/protocols/core/Shm.hpp b/src/protocols/core/Shm.hpp index e504146d..37210398 100644 --- a/src/protocols/core/Shm.hpp +++ b/src/protocols/core/Shm.hpp @@ -20,6 +20,7 @@ class CWLSHMPoolResource; class CSHMPool { public: + CSHMPool() = delete; CSHMPool(Hyprutils::OS::CFileDescriptor fd, size_t size); ~CSHMPool(); @@ -32,7 +33,7 @@ class CSHMPool { class CWLSHMBuffer : public IHLBuffer { public: - CWLSHMBuffer(SP pool, uint32_t id, int32_t offset, const Vector2D& size, int32_t stride, uint32_t fmt); + CWLSHMBuffer(WP pool, uint32_t id, int32_t offset, const Vector2D& size, int32_t stride, uint32_t fmt); virtual ~CWLSHMBuffer(); virtual Aquamarine::eBufferCapability caps(); @@ -58,7 +59,7 @@ class CWLSHMBuffer : public IHLBuffer { class CWLSHMPoolResource { public: - CWLSHMPoolResource(SP resource_, Hyprutils::OS::CFileDescriptor fd, size_t size); + CWLSHMPoolResource(UP&& resource_, Hyprutils::OS::CFileDescriptor fd, size_t size); bool good(); @@ -67,19 +68,19 @@ class CWLSHMPoolResource { WP m_self; private: - SP m_resource; + UP m_resource; friend class CWLSHMBuffer; }; class CWLSHMResource { public: - CWLSHMResource(SP resource_); + CWLSHMResource(UP&& resource_); bool good(); private: - SP m_resource; + UP m_resource; }; class CWLSHMProtocol : public IWaylandProtocol { @@ -94,8 +95,8 @@ class CWLSHMProtocol : public IWaylandProtocol { void destroyResource(CWLSHMBuffer* resource); // - std::vector> m_managers; - std::vector> m_pools; + std::vector> m_managers; + std::vector> m_pools; std::vector> m_buffers; //