From 87fe9719c1f306537b5025b3d298623074f5b40c Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Mon, 9 Oct 2023 16:20:26 -0700 Subject: [PATCH] Fix memory leak, start work on win file api. --- Arena.cc | 110 +++++++++++++++++++++++++--------------------- Arena.h | 12 ++--- bufferTest.cc | 23 +++++----- dictionaryTest.cc | 8 ++-- tlvTest.cc | 6 +-- 5 files changed, 84 insertions(+), 75 deletions(-) diff --git a/Arena.cc b/Arena.cc index 88a233c..ae1fe72 100644 --- a/Arena.cc +++ b/Arena.cc @@ -4,11 +4,16 @@ #include #if defined(__linux__) + #include #include #include #include #include + +#define PROT_RW (PROT_WRITE|PROT_READ) +#elif defined(__WIN64__) || defined(__WIN32__) +#include #endif #include @@ -71,79 +76,81 @@ Arena::SetAlloc(size_t allocSize) #if defined(__linux__) + int -MMapArena(Arena &arena, int fd, size_t size) +Arena::MemoryMap(int memFileDes, size_t memSize) { if (this->size > 0) { - if (DestroyArena(arena) != 0) { - return -1; - } + this->Destroy(); } this->arenaType = ARENA_MMAP; - this->size = size; - this->store = (uint8_t *)mmap(NULL, size, PROT_RW, MAP_SHARED, fd, 0); - if ((void *)this->store == MAP_FAILED) { + this->size = memSize; + this->store = (uint8_t *) mmap(NULL, memSize, PROT_RW, MAP_SHARED, + memFileDes, 0); + if ((void *) this->store == MAP_FAILED) { return -1; } - this->fd = fd; + this->fd = memFileDes; return 0; } int -OpenArena(Arena &arena, const char *path) +Arena::Open(const char *path) { - struct stat st; + struct stat st{}; if (this->size > 0) { - if (DestroyArena(arena) != 0) { - return -1; - } + this->Destroy(); } if (stat(path, &st) != 0) { return -1; } - this->fd = open(path, O_RDWR); + this->fd = open(path, O_RDWR); if (this->fd == -1) { return -1; } - return MMapArena(arena, this->fd, (size_t)st.st_size); + return this->MemoryMap(this->fd, (size_t) st.st_size); } int -CreateArena(Arena &arena, const char *path, size_t size, mode_t mode) +Arena::Create(const char *path, size_t fileSize, mode_t mode) { - int fd = 0; + int newFileDes = 0; if (this->size > 0) { - if (DestroyArena(arena) != 0) { - return -1; - } + this->Destroy(); } - fd = open(path, O_WRONLY|O_CREAT|O_TRUNC, mode); - if (fd == -1) { + newFileDes = open(path, O_WRONLY | O_CREAT | O_TRUNC, mode); + if (newFileDes == -1) { return -1; } - if (ftruncate(fd, size) == -1) { + if (ftruncate(newFileDes, fileSize) == -1) { return -1; } - close(fd); + close(newFileDes); + + return this->Open(path); +} +#elif defined(__WIN64__) || defined(__WIN32__) +int +Arena::Open(const char *path) +{ - return OpenArena(arena, path); } #endif bool -Arena::CursorInArena(uint8_t *cursor) +Arena::CursorInArena(const uint8_t *cursor) { if (cursor < this->store) { return false; @@ -186,25 +193,26 @@ Arena::Destroy() case ARENA_ALLOC: delete this->store; break; - #if defined(__linux__) - case ARENA_MMAP: - if (munmap(this->store, this->size) == -1) { - return -1; - } +#if defined(__linux__) + case ARENA_MMAP: + if (munmap(this->store, this->size) == -1) { + abort(); + return; + } - if (close(this->fd) == -1) { - return -1; - } + if (close(this->fd) == -1) { + abort(); + } - this->fd = 0; - break; - #endif + this->fd = 0; + break; +#endif default: - #if defined(NDEBUG) +#if defined(NDEBUG) return -1; - #else +#else abort(); - #endif +#endif } @@ -215,11 +223,11 @@ Arena::Destroy() } std::ostream & -operator<<(std::ostream &os, const Arena arena) +operator<<(std::ostream &os, Arena &arena) { - auto cursor = arena.NewCursor(); - char cursorString[17] = {0}; - snprintf(cursorString, 16, "%0p", cursor); + auto cursor = arena.Store(); + char cursorString[33] = {0}; + snprintf(cursorString, 32, "%#016lx", cursor); os << "Arena<"; switch (arena.Type()) { @@ -233,9 +241,9 @@ operator<<(std::ostream &os, const Arena arena) os << "allocated"; break; #if defined(__linux__) - case ARENA_MMAP: - os << "mmap/file"; - break; + case ARENA_MMAP: + os << "mmap/file"; + break; #endif default: os << "unknown (this is a bug)"; @@ -243,7 +251,7 @@ operator<<(std::ostream &os, const Arena arena) os << ">@0x"; os << std::hex << (uintptr_t) &arena; os << std::dec; - os << ",store<" << arena.Size() << "B>@0x"; + os << ",store<" << arena.Size() << "B>@"; os << std::hex << cursorString; os << std::dec; @@ -257,11 +265,11 @@ Arena::Write(const char *path) FILE *arenaFile = NULL; int retc = -1; -#if defined(__posix__) +#if defined(__posix__) || defined(__linux__) arenaFile = fopen(path, "w"); - if (arenaFile == NULL) { + if (arenaFile == nullptr) { #else - if (fopen_s(&arenaFile, path, "w") != 0) { + if (fopen_s(&arenaFile, path, "w") != 0) { #endif return -1; } diff --git a/Arena.h b/Arena.h index 69f0735..e02e237 100644 --- a/Arena.h +++ b/Arena.h @@ -38,14 +38,14 @@ public: int SetAlloc(size_t allocSize); #if defined(__linux__) - int MemoryMap(int fd); // Arena will own fd. - int Create(const char *path, size_t size, mode_t mode); - int Open(const char *, size_t); + int MemoryMap(int memFileDes, size_t memSize); // Arena will own fd. + int Create(const char *path, size_t fileSize, mode_t mode); + int Open(const char *path); #endif uint8_t *NewCursor() const { return this->store; } uint8_t *End() { return this->store + this->size; } - bool CursorInArena(uint8_t *cursor); + bool CursorInArena(const uint8_t *cursor); size_t Size() const { return this->size; } @@ -53,6 +53,8 @@ public: uint8_t Type() const { return this->arenaType; } + uintptr_t Store() { return (uintptr_t)this->store; } + bool Ready() const { return this->Type() != ARENA_UNINIT; }; void Clear(); void Destroy(); /* dispose of any memory used by arena */ @@ -74,7 +76,7 @@ private: }; -std::ostream &operator<<(std::ostream& os, Arena arena); +std::ostream &operator<<(std::ostream& os, Arena &arena); } // namespace klib diff --git a/bufferTest.cc b/bufferTest.cc index 02c79e9..9048159 100644 --- a/bufferTest.cc +++ b/bufferTest.cc @@ -6,32 +6,33 @@ int main(int argc, char *argv[]) { - (void)argc; (void)argv; + (void) argc; + (void) argv; - klib::Buffer buffer("hlo, world"); + klib::Buffer buffer("hlo, world"); std::cout << buffer.Contents() << std::endl; - buffer.Insert(1, (uint8_t *)"el", 2); + buffer.Insert(1, (uint8_t *) "el", 2); buffer.Append('!'); std::cout << buffer.Contents() << std::endl; - buffer.Remove(buffer.Length()-1); + buffer.Remove(buffer.Length() - 1); buffer.Remove(0, 5); buffer.Insert(0, 'g'); - buffer.Insert(1, (uint8_t *)"oodbye", 6); + buffer.Insert(1, (uint8_t *) "oodbye", 6); buffer.Reclaim(); buffer.Append("and now for something completely different..."); std::cout << buffer.Contents() << std::endl; - std::cout << "Length: " << buffer.Length() << ", capacity " << buffer.Capacity() << std::endl; - buffer.Resize(128); - std::cout << "Length: " << buffer.Length() << ", capacity " << buffer.Capacity() << std::endl; - buffer.Trim(); - std::cout << "Length: " << buffer.Length() << ", capacity " << buffer.Capacity() << std::endl; + std::cout << "Length: " << buffer.Length() << ", capacity " << buffer.Capacity() << std::endl; + buffer.Resize(128); + std::cout << "Length: " << buffer.Length() << ", capacity " << buffer.Capacity() << std::endl; + buffer.Trim(); + std::cout << "Length: " << buffer.Length() << ", capacity " << buffer.Capacity() << std::endl; - return 0; + return 0; } diff --git a/dictionaryTest.cc b/dictionaryTest.cc index 7e9e537..b7ef42c 100644 --- a/dictionaryTest.cc +++ b/dictionaryTest.cc @@ -42,10 +42,9 @@ main(int argc, const char *argv[]) TLV::Record expect; std::cout << "TESTPROG: " << argv[0] << std::endl; - InitializeArena(arena); #if defined(__linux__) - if (CreateArena(arena, ARENA_FILE, ARENA_SIZE, 0644) == -1) { + if (arena.Create(ARENA_FILE, ARENA_SIZE, 0644) == -1) { abort(); } #else @@ -53,8 +52,7 @@ main(int argc, const char *argv[]) abort(); } #endif - DisplayArena(arena); - + std::cout << arena << std::endl; TLV::SetRecord(expect, DICTIONARY_TAG_VAL, TEST_KVSTRLEN3, TEST_KVSTR3); Dictionary dict(arena); @@ -101,6 +99,6 @@ main(int argc, const char *argv[]) dict.DumpToFile(ARENA_FILE); #endif - ClearArena(arena); + arena.Clear(); dict.DumpKVPairs(); } diff --git a/tlvTest.cc b/tlvTest.cc index a646c67..f1a40da 100644 --- a/tlvTest.cc +++ b/tlvTest.cc @@ -1,5 +1,6 @@ #include #include +#include #include #include "Arena.h" @@ -62,7 +63,7 @@ runSuite(Arena &backend, const char *label) std::cout << "running test suite " << label << ": "; try { tlvTestSuite(backend); - } catch (_exception){ + } catch (std::exception &exc){ std::cout << "FAILED" << std::endl; return false; } @@ -95,9 +96,8 @@ main(int argc, const char *argv[]) #if defined(__linux__) Arena arenaFile; - InitializeArena(arenaFile); - if (-1 == CreateArena(arenaFile, ARENA_FILE, ARENA_SIZE, 0644)) { + if (-1 == arenaFile.Create(ARENA_FILE, ARENA_SIZE, 0644)) { abort(); } else if (!runSuite(arenaFile, "arenaFile")) { abort();