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.
This commit is contained in:
Tom Englund 2025-09-25 01:44:33 +02:00 committed by GitHub
parent 683fc77f80
commit 8cce3b98ce
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 22 additions and 16 deletions

View file

@ -9,7 +9,10 @@
#include "../../render/Renderer.hpp" #include "../../render/Renderer.hpp"
using namespace Hyprutils::OS; using namespace Hyprutils::OS;
CWLSHMBuffer::CWLSHMBuffer(SP<CWLSHMPoolResource> pool_, uint32_t id, int32_t offset_, const Vector2D& size_, int32_t stride_, uint32_t fmt_) { CWLSHMBuffer::CWLSHMBuffer(WP<CWLSHMPoolResource> 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) if UNLIKELY (!pool_->m_pool->m_data)
return; return;
@ -79,14 +82,16 @@ CSHMPool::CSHMPool(CFileDescriptor fd_, size_t size_) : m_fd(std::move(fd_)), m_
} }
CSHMPool::~CSHMPool() { CSHMPool::~CSHMPool() {
munmap(m_data, m_size); if (m_data != MAP_FAILED)
munmap(m_data, m_size);
} }
void CSHMPool::resize(size_t size_) { void CSHMPool::resize(size_t size_) {
LOGM(LOG, "Resizing a SHM pool from {} to {}", m_size, 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); munmap(m_data, m_size);
m_size = size_; m_size = size_;
m_data = mmap(nullptr, m_size, PROT_READ | PROT_WRITE, MAP_SHARED, m_fd.get(), 0); 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<size_t>(st.st_size) >= size; return sc<size_t>(st.st_size) >= size;
} }
CWLSHMPoolResource::CWLSHMPoolResource(SP<CWlShmPool> resource_, CFileDescriptor fd_, size_t size_) : m_resource(resource_) { CWLSHMPoolResource::CWLSHMPoolResource(UP<CWlShmPool>&& resource_, CFileDescriptor fd_, size_t size_) : m_resource(std::move(resource_)) {
if UNLIKELY (!good()) if UNLIKELY (!good())
return; return;
if UNLIKELY (!shmIsSizeValid(fd_, size_)) { 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; return;
} }
@ -147,7 +152,7 @@ CWLSHMPoolResource::CWLSHMPoolResource(SP<CWlShmPool> resource_, CFileDescriptor
return; return;
} }
const auto RESOURCE = PROTO::shm->m_buffers.emplace_back(makeShared<CWLSHMBuffer>(m_self.lock(), id, offset, Vector2D{w, h}, stride, fmt)); const auto& RESOURCE = PROTO::shm->m_buffers.emplace_back(makeShared<CWLSHMBuffer>(m_self, id, offset, Vector2D{w, h}, stride, fmt));
if UNLIKELY (!RESOURCE->good()) { if UNLIKELY (!RESOURCE->good()) {
r->noMemory(); r->noMemory();
@ -167,7 +172,7 @@ bool CWLSHMPoolResource::good() {
return m_resource->resource(); return m_resource->resource();
} }
CWLSHMResource::CWLSHMResource(SP<CWlShm> resource_) : m_resource(resource_) { CWLSHMResource::CWLSHMResource(UP<CWlShm>&& resource_) : m_resource(std::move(resource_)) {
if UNLIKELY (!good()) if UNLIKELY (!good())
return; return;
@ -175,7 +180,7 @@ CWLSHMResource::CWLSHMResource(SP<CWlShm> resource_) : m_resource(resource_) {
m_resource->setCreatePool([](CWlShm* r, uint32_t id, int32_t fd, int32_t size) { m_resource->setCreatePool([](CWlShm* r, uint32_t id, int32_t fd, int32_t size) {
CFileDescriptor poolFd{fd}; CFileDescriptor poolFd{fd};
const auto RESOURCE = PROTO::shm->m_pools.emplace_back(makeShared<CWLSHMPoolResource>(makeShared<CWlShmPool>(r->client(), r->version(), id), std::move(poolFd), size)); const auto& RESOURCE = PROTO::shm->m_pools.emplace_back(makeUnique<CWLSHMPoolResource>(makeUnique<CWlShmPool>(r->client(), r->version(), id), std::move(poolFd), size));
if UNLIKELY (!RESOURCE->good()) { if UNLIKELY (!RESOURCE->good()) {
r->noMemory(); 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<CWLSHMResource>(makeShared<CWlShm>(client, ver, id))); const auto& RESOURCE = m_managers.emplace_back(makeUnique<CWLSHMResource>(makeUnique<CWlShm>(client, ver, id)));
if UNLIKELY (!RESOURCE->good()) { if UNLIKELY (!RESOURCE->good()) {
wl_client_post_no_memory(client); wl_client_post_no_memory(client);

View file

@ -20,6 +20,7 @@ class CWLSHMPoolResource;
class CSHMPool { class CSHMPool {
public: public:
CSHMPool() = delete;
CSHMPool(Hyprutils::OS::CFileDescriptor fd, size_t size); CSHMPool(Hyprutils::OS::CFileDescriptor fd, size_t size);
~CSHMPool(); ~CSHMPool();
@ -32,7 +33,7 @@ class CSHMPool {
class CWLSHMBuffer : public IHLBuffer { class CWLSHMBuffer : public IHLBuffer {
public: public:
CWLSHMBuffer(SP<CWLSHMPoolResource> pool, uint32_t id, int32_t offset, const Vector2D& size, int32_t stride, uint32_t fmt); CWLSHMBuffer(WP<CWLSHMPoolResource> pool, uint32_t id, int32_t offset, const Vector2D& size, int32_t stride, uint32_t fmt);
virtual ~CWLSHMBuffer(); virtual ~CWLSHMBuffer();
virtual Aquamarine::eBufferCapability caps(); virtual Aquamarine::eBufferCapability caps();
@ -58,7 +59,7 @@ class CWLSHMBuffer : public IHLBuffer {
class CWLSHMPoolResource { class CWLSHMPoolResource {
public: public:
CWLSHMPoolResource(SP<CWlShmPool> resource_, Hyprutils::OS::CFileDescriptor fd, size_t size); CWLSHMPoolResource(UP<CWlShmPool>&& resource_, Hyprutils::OS::CFileDescriptor fd, size_t size);
bool good(); bool good();
@ -67,19 +68,19 @@ class CWLSHMPoolResource {
WP<CWLSHMPoolResource> m_self; WP<CWLSHMPoolResource> m_self;
private: private:
SP<CWlShmPool> m_resource; UP<CWlShmPool> m_resource;
friend class CWLSHMBuffer; friend class CWLSHMBuffer;
}; };
class CWLSHMResource { class CWLSHMResource {
public: public:
CWLSHMResource(SP<CWlShm> resource_); CWLSHMResource(UP<CWlShm>&& resource_);
bool good(); bool good();
private: private:
SP<CWlShm> m_resource; UP<CWlShm> m_resource;
}; };
class CWLSHMProtocol : public IWaylandProtocol { class CWLSHMProtocol : public IWaylandProtocol {
@ -94,8 +95,8 @@ class CWLSHMProtocol : public IWaylandProtocol {
void destroyResource(CWLSHMBuffer* resource); void destroyResource(CWLSHMBuffer* resource);
// //
std::vector<SP<CWLSHMResource>> m_managers; std::vector<UP<CWLSHMResource>> m_managers;
std::vector<SP<CWLSHMPoolResource>> m_pools; std::vector<UP<CWLSHMPoolResource>> m_pools;
std::vector<SP<CWLSHMBuffer>> m_buffers; std::vector<SP<CWLSHMBuffer>> m_buffers;
// //