diff --git a/plugins/link/LinkedMem.h b/plugins/link/LinkedMem.h index e6472c5db..e83d9da01 100644 --- a/plugins/link/LinkedMem.h +++ b/plugins/link/LinkedMem.h @@ -19,34 +19,36 @@ #endif // NOTE: To prevent possible undefined behavior in -// `SharedMemory::write`, this struct must not -// have any padding. +// `SharedMemory::write` and `SharedMemory::reset`, this +// struct must not have any padding, and no members may +// have indeterminate bits when this struct is +// default-constructed. struct LinkedMem { #ifdef _WIN32 - UINT32 uiVersion; - DWORD uiTick; + UINT32 uiVersion = 0; + DWORD uiTick = 0; #else - uint32_t uiVersion; - uint32_t uiTick; + uint32_t uiVersion = 0; + uint32_t uiTick = 0; #endif - float fAvatarPosition[3]; - float fAvatarFront[3]; - float fAvatarTop[3]; - wchar_t name[256]; - float fCameraPosition[3]; - float fCameraFront[3]; - float fCameraTop[3]; - wchar_t identity[256]; + float fAvatarPosition[3] = { 0 }; + float fAvatarFront[3] = { 0 }; + float fAvatarTop[3] = { 0 }; + wchar_t name[256] = { 0 }; + float fCameraPosition[3] = { 0 }; + float fCameraFront[3] = { 0 }; + float fCameraTop[3] = { 0 }; + wchar_t identity[256] = { 0 }; #ifdef _WIN32 - UINT32 context_len; + UINT32 context_len = 0; #else - uint32_t context_len; + uint32_t context_len = 0; #endif - unsigned char context[256]; - wchar_t description[2048]; + unsigned char context[256] = { 0 }; + wchar_t description[2048] = { 0 }; }; -static const char *getLinkedMemoryName() { +static inline const char *getLinkedMemoryName() { #ifdef _WIN32 return "MumbleLink"; #else diff --git a/plugins/link/SharedMemory.cpp b/plugins/link/SharedMemory.cpp index e8b4cc916..93f099f9e 100644 --- a/plugins/link/SharedMemory.cpp +++ b/plugins/link/SharedMemory.cpp @@ -5,6 +5,8 @@ #include "SharedMemory.h" +#include "LinkedMem.h" + #ifdef _WIN32 #else // Note: Linking to the "rt" library is required @@ -22,8 +24,6 @@ #include -#include - // A Chunk is one atomically accessed chunk of shared memory. // // The only way to check at compile time if operations on a @@ -43,11 +43,14 @@ static_assert(std::is_same< int, std::int32_t >::value, "int isn't the same as s static_assert(ATOMIC_INT_LOCK_FREE == 2, "std::atomic< int > may not be lock-free"); using Chunk = int; -static constexpr std::size_t chunkSize = sizeof(Chunk); +static constexpr std::size_t chunkSize = sizeof(Chunk); +static constexpr std::size_t chunkCount = sizeof(LinkedMem) / chunkSize; + static_assert(sizeof(std::atomic< Chunk >) == chunkSize, "std::atomic< Chunk > has a size different from Chunk's"); +static_assert(chunkSize * chunkCount == sizeof(LinkedMem), "LinkedMem's size isn't a multiple of Chunk's size"); SharedMemory::SharedMemory() - : m_data(nullptr), m_size(0), m_error(0), + : m_data(nullptr), m_error(0), #ifdef _WIN32 m_handle(NULL) #else @@ -60,10 +63,6 @@ SharedMemory::~SharedMemory() { close(); } -std::size_t SharedMemory::size() { - return m_size; -} - void SharedMemory::close() { #ifdef _WIN32 if (m_data) { @@ -76,7 +75,7 @@ void SharedMemory::close() { m_handle = NULL; #else if (m_data) { - munmap(m_data, m_size); + munmap(m_data, sizeof(LinkedMem)); } if (!m_name.empty()) { shm_unlink(m_name.c_str()); @@ -86,7 +85,6 @@ void SharedMemory::close() { #endif m_data = nullptr; - m_size = 0; m_error = 0; } @@ -94,10 +92,7 @@ int SharedMemory::lastError() const { return m_error; } -bool SharedMemory::mapMemory(const char *name, std::size_t size) { - // Round size up to the nearest multiple of the chunk size - size = (size + chunkSize - 1) / chunkSize * chunkSize; - +bool SharedMemory::mapMemory(const char *name) { close(); bool created = false; @@ -107,7 +102,7 @@ bool SharedMemory::mapMemory(const char *name, std::size_t size) { if (m_handle == NULL) { // Attaching failed, so we have to create it - m_handle = CreateFileMappingA(INVALID_HANDLE_VALUE, nullptr, PAGE_READWRITE, 0, size, name); + m_handle = CreateFileMappingA(INVALID_HANDLE_VALUE, nullptr, PAGE_READWRITE, 0, sizeof(LinkedMem), name); if (m_handle == NULL) { m_error = GetLastError(); @@ -144,8 +139,8 @@ bool SharedMemory::mapMemory(const char *name, std::size_t size) { return false; } - // Truncate to specified size - if (ftruncate(fd, size) != 0) { + // Truncate to correct size + if (ftruncate(fd, sizeof(LinkedMem)) != 0) { m_error = errno; ::close(fd); @@ -156,7 +151,7 @@ bool SharedMemory::mapMemory(const char *name, std::size_t size) { created = true; } - m_data = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + m_data = mmap(nullptr, sizeof(LinkedMem), PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if (m_data == reinterpret_cast< void * >(-1)) { m_data = nullptr; @@ -176,49 +171,45 @@ bool SharedMemory::mapMemory(const char *name, std::size_t size) { m_name.assign(name); #endif - m_size = size; - if (created) { - fillWithZero(); + reset(); } return true; } -bool SharedMemory::isMemoryMapped() { +bool SharedMemory::isMemoryMapped() const { return m_data != nullptr; } -void SharedMemory::read(void *dest, std::size_t size) { +LinkedMem SharedMemory::read() const { const auto *data = static_cast< const std::atomic< Chunk > * >(m_data); - std::vector< Chunk > buff(m_size / chunkSize, 0); + Chunk buff[chunkCount]; - for (std::size_t i = 0; i < m_size / chunkSize; i++) { + for (std::size_t i = 0; i < chunkCount; i++) { buff[i] = data[i].load(std::memory_order_relaxed); } // bitcast the chunks with memcpy to avoid running afoul // of type-based alias analysis - std::memcpy(dest, &buff[0], std::min(size, m_size)); + LinkedMem dest; + std::memcpy(&dest, buff, sizeof(LinkedMem)); + return dest; } -void SharedMemory::write(const void *source, std::size_t size) { +void SharedMemory::write(const LinkedMem &source) { auto *data = static_cast< std::atomic< Chunk > * >(m_data); - std::vector< Chunk > buff(m_size / chunkSize, 0); + Chunk buff[chunkCount]; - std::memcpy(&buff[0], source, std::min(size, m_size)); + std::memcpy(buff, &source, sizeof(LinkedMem)); - for (std::size_t i = 0; i < m_size / chunkSize; i++) { + for (std::size_t i = 0; i < chunkCount; i++) { data[i].store(buff[i], std::memory_order_relaxed); } } -void SharedMemory::fillWithZero() { - auto *data = static_cast< std::atomic< Chunk > * >(m_data); - - for (std::size_t i = 0; i < m_size / chunkSize; i++) { - data[i].store(0, std::memory_order_relaxed); - } +void SharedMemory::reset() { + write(LinkedMem()); } diff --git a/plugins/link/SharedMemory.h b/plugins/link/SharedMemory.h index 4773627e0..f5b23df8f 100644 --- a/plugins/link/SharedMemory.h +++ b/plugins/link/SharedMemory.h @@ -17,30 +17,29 @@ # include #endif +struct LinkedMem; + class SharedMemory { public: explicit SharedMemory(); ~SharedMemory(); - std::size_t size(); - void close(); int lastError() const; - bool mapMemory(const char *name, std::size_t size); + bool mapMemory(const char *name); - bool isMemoryMapped(); + bool isMemoryMapped() const; - void read(void *dest, std::size_t size); + LinkedMem read() const; - void write(const void *source, std::size_t size); + void write(const LinkedMem &source); - void fillWithZero(); + void reset(); private: void *m_data; - std::size_t m_size; int m_error; #ifdef _WIN32 diff --git a/plugins/link/link.cpp b/plugins/link/link.cpp index 5eefc2e9c..934f049a5 100644 --- a/plugins/link/link.cpp +++ b/plugins/link/link.cpp @@ -43,7 +43,7 @@ static std::uint64_t getTimeSinceEpoch() { mumble_error_t mumble_init(mumble_plugin_id_t id) { UNUSED(id); - if (!sharedMem.mapMemory(getLinkedMemoryName(), sizeof(LinkedMem))) { + if (!sharedMem.mapMemory(getLinkedMemoryName())) { std::cerr << "Link plugin: Failed to setup shared memory: " << sharedMem.lastError() << std::endl; return MUMBLE_EC_INTERNAL_ERROR; @@ -117,8 +117,7 @@ uint8_t mumble_initPositionalData(const char *const *programNames, const uint64_ return MUMBLE_PDEC_ERROR_TEMP; } - LinkedMem lm; - sharedMem.read(&lm, sizeof(lm)); + LinkedMem lm = sharedMem.read(); if ((lm.uiVersion == 1) || (lm.uiVersion == 2)) { if (lm.uiTick != last_tick) { @@ -162,8 +161,7 @@ bool mumble_fetchPositionalData(float *avatarPos, float *avatarDir, float *avata SET_TO_ZERO(cameraDir); SET_TO_ZERO(cameraAxis); - LinkedMem lm; - sharedMem.read(&lm, sizeof(lm)); + LinkedMem lm = sharedMem.read(); if (lm.uiTick != last_tick) { last_tick = lm.uiTick; @@ -231,7 +229,7 @@ void mumble_shutdownPositionalData() { pluginContext.clear(); pluginIdentity.clear(); - sharedMem.fillWithZero(); + sharedMem.reset(); } MumbleStringWrapper mumble_getPositionalDataContextPrefix() { diff --git a/plugins/link/link_tester.cpp b/plugins/link/link_tester.cpp index 340e1352a..f3d3e64eb 100644 --- a/plugins/link/link_tester.cpp +++ b/plugins/link/link_tester.cpp @@ -25,9 +25,9 @@ std::mt19937 rng(dev()); std::uniform_real_distribution< float > generator(0, 100); void initMumble() { - sharedMem.mapMemory(getLinkedMemoryName(), sizeof(LinkedMem)); + sharedMem.mapMemory(getLinkedMemoryName()); - std::memset(&lm, 0, sizeof(lm)); + lm = LinkedMem(); } void updateMumble() { @@ -83,7 +83,7 @@ void updateMumble() { memcpy(lm.context, "ContextBlob\x00\x01\x02\x03\x04", 16); lm.context_len = 16; - sharedMem.write(&lm, sizeof(lm)); + sharedMem.write(lm); } void signalHandler(int signum) {