From bf1ea5e74995bc52d58915576afcb7681a7e0814 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Tue, 10 Oct 2023 18:57:43 -0700 Subject: [PATCH] Code cleanups. --- Arena.cc | 15 +++--- Arena.h | 4 +- Buffer.cc | 2 + CMakeLists.txt | 14 +++-- Commander.cpp => Commander.cc | 0 Exceptions.cpp => Exceptions.cc | 4 +- Exceptions.h | 4 +- Test.cc | 26 ++++++++-- Test.h | 13 ++++- dictionaryTest.cc | 91 +++++++++++++++++---------------- 10 files changed, 108 insertions(+), 65 deletions(-) rename Commander.cpp => Commander.cc (100%) rename Exceptions.cpp => Exceptions.cc (80%) diff --git a/Arena.cc b/Arena.cc index 723b324..1072c67 100644 --- a/Arena.cc +++ b/Arena.cc @@ -3,8 +3,7 @@ #include #include -#if defined(__linux__) - +#if defined(__posix__) || defined(__linux__) || defined(__APPLE__) #include #include #include @@ -66,8 +65,7 @@ Arena::SetAlloc(size_t allocSize) } -#if defined(__linux__) - +#if defined(__posix__) || defined(__linux__) || defined(__APPLE__) int Arena::MemoryMap(int memFileDes, size_t memSize) { @@ -205,6 +203,7 @@ Arena::Create(const char *path, size_t fileSize) #endif + bool Arena::CursorInArena(const uint8_t *cursor) { @@ -249,7 +248,7 @@ Arena::Destroy() case ArenaType::Alloc: delete this->store; break; -#if defined(__linux__) +#if defined(__posix__) || defined(__linux__) || defined(__APPLE__) case ArenaType::MemoryMapped: if (munmap(this->store, this->size) == -1) { abort(); @@ -265,7 +264,7 @@ Arena::Destroy() #endif default: #if defined(NDEBUG) - return -1; + return; #else abort(); #endif @@ -297,7 +296,7 @@ operator<<(std::ostream &os, Arena &arena) case ArenaType::Alloc: os << "allocated"; break; -#if defined(__linux__) +#if defined(__posix__) || defined(__linux__) || defined(__APPLE__) case ArenaType::MemoryMapped: os << "mmap/file"; break; @@ -322,7 +321,7 @@ Arena::Write(const char *path) FILE *arenaFile = nullptr; int retc = -1; -#if defined(__posix__) || defined(__linux__) +#if defined(__posix__) || defined(__linux__) || defined(__APPLE__) arenaFile = fopen(path, "w"); if (arenaFile == nullptr) { #else diff --git a/Arena.h b/Arena.h index 3aa78a9..07e507a 100644 --- a/Arena.h +++ b/Arena.h @@ -96,7 +96,7 @@ public: /// \param memFileDes File descriptor to map into memory. /// \param memSize The size of memory to map. /// \return Returns 0 on success and -1 on error. -#if defined(__linux__) +#if defined(__posix__) || defined(__linux__) || defined(__APPLE__) int MemoryMap(int memFileDes, size_t memSize); #else @@ -112,7 +112,7 @@ public: /// \param path The path to the file that should be created. /// \param fileSize The size of the file to create. /// \return Returns 0 on success and -1 on error. -#if defined(__linux__) +#if defined(__posix__) || defined(__linux__) || defined(__APPLE__) int Create(const char *path, size_t fileSize); #elif defined(__WIN64__) || defined(__WIN32__) || defined(WIN32) int Create(const char *path, size_t fileSize); diff --git a/Buffer.cc b/Buffer.cc index ba1ee12..1da5bbf 100644 --- a/Buffer.cc +++ b/Buffer.cc @@ -275,6 +275,8 @@ Buffer::HexDump(std::ostream &os) } os << std::setw(0) << std::dec; +#else + (void)os; #endif } diff --git a/CMakeLists.txt b/CMakeLists.txt index 206a16b..cadf2d0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,7 +10,15 @@ set(VERBOSE YES) if (MSVC) add_compile_options("/W4" "$<$:/O2>") else () - add_compile_options("-Wall" "-Wextra" "-Werror" "$<$:-O3>") + # compile options: + # -Wall Default to all errors. + # -Wextra And a few extra. + # -Werror And require them to be fixed to build. + # -Wno-unused-function This is a library. Not every function is used here. + # -Wno-unused-parameter Some functions have parameters defined for compatibility, + # and aren't used in the implementation. + + add_compile_options("-Wall" "-Wextra" "-Werror" "-Wno-unused-function" "-Wno-unused-parameter" "$<$:-O2>") if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") add_compile_options("-stdlib=libc++") else () @@ -33,10 +41,10 @@ set(SOURCE_FILES Arena.cc Buffer.cc Dictionary.cc - Exceptions.cpp + Exceptions.cc Test.cc TLV.cc - Commander.cpp + Commander.cc Commander.h WinHelpers.cc) diff --git a/Commander.cpp b/Commander.cc similarity index 100% rename from Commander.cpp rename to Commander.cc diff --git a/Exceptions.cpp b/Exceptions.cc similarity index 80% rename from Exceptions.cpp rename to Exceptions.cc index 9bd5274..3c04ec1 100644 --- a/Exceptions.cpp +++ b/Exceptions.cc @@ -12,8 +12,8 @@ AssertionFailed::AssertionFailed(std::string message) : msg(message) { } -char * -AssertionFailed::what() +const char * +AssertionFailed::what() const throw() { return const_cast(this->msg.c_str()); } diff --git a/Exceptions.h b/Exceptions.h index ee2cd56..fbeb561 100644 --- a/Exceptions.h +++ b/Exceptions.h @@ -22,7 +22,7 @@ public: explicit NotImplemented(const char *pl) : platform((char *)pl) {} /// what returns a message naming the platform. - char *what() { + const char *what() const throw() { return this->platform; } private: @@ -38,7 +38,7 @@ public: explicit AssertionFailed(std::string message); /// what returns a message describing the exception. - char *what(); + const char *what() const throw(); private: std::string msg; diff --git a/Test.cc b/Test.cc index f0637bd..daab849 100644 --- a/Test.cc +++ b/Test.cc @@ -5,14 +5,13 @@ #include "Exceptions.h" #include "Test.h" - -#include #include +#include +#include namespace klib { - void TestAssert(bool condition, std::string message = "Assertion failed.") { @@ -29,4 +28,25 @@ TestAssert(bool condition, std::string message = "Assertion failed.") } +void +TestAssert(bool condition) +{ +#if defined(NDEBUG) + if (condition) { + return; + } +#if defined(KLIB_NO_ASSERT) + std::cerr << "Assertion failed!\n"; +#else + std::stringstream msg; + + msg << "assertion failed at " << __FILE__ << ":" << __LINE__; + throw AssertionFailed(msg.str()); +#endif +#else + assert(condition); +#endif +} + + } // namespace klib diff --git a/Test.h b/Test.h index 23b810d..04f88f8 100644 --- a/Test.h +++ b/Test.h @@ -13,6 +13,17 @@ namespace klib { +/// TestAssert is a variant on the assert macro. This variant is intended to be +/// a drop-in replacement for the cassert macro: even in release mode, the tests +/// should still run. +/// +/// If NDEBUG is set, TestAssert will throw an exception if condition is false. +/// Otherwise, it calls assert after printing the message. +/// +/// \param condition If true, TestAssert throws an exception. +void TestAssert(bool condition); + + /// TestAssert is a variant on the assert macro. /// /// If NDEBUG is set, TestAssert will throw an exception if condition is false. @@ -24,7 +35,7 @@ namespace klib { /// /// \param condition The condition to assert. /// \param message The message that should be displayed if condition is false. -inline void TestAssert(bool condition, std::string message); +void TestAssert(bool condition, std::string message); } // namespace klib diff --git a/dictionaryTest.cc b/dictionaryTest.cc index 7161925..a242c5a 100644 --- a/dictionaryTest.cc +++ b/dictionaryTest.cc @@ -1,31 +1,33 @@ -#include #include #include "Arena.h" #include "Dictionary.h" +#include "Test.h" #include "testFixtures.h" + + using namespace klib; -constexpr char TEST_KVSTR1[] = "foo"; -constexpr uint8_t TEST_KVSTRLEN1 = 3; -constexpr char TEST_KVSTR2[] = "baz"; -constexpr uint8_t TEST_KVSTRLEN2 = 3; -constexpr char TEST_KVSTR3[] = "quux"; -constexpr uint8_t TEST_KVSTRLEN3 = 4; -constexpr char TEST_KVSTR4[] = "spam"; -constexpr uint8_t TEST_KVSTRLEN4 = 4; -constexpr char TEST_KVSTR5[] = "xyzzx"; -constexpr uint8_t TEST_KVSTRLEN5 = 5; -constexpr char TEST_KVSTR6[] = "corvid"; -constexpr uint8_t TEST_KVSTRLEN6 = 6; +constexpr char TEST_KVSTR1[] = "foo"; +constexpr uint8_t TEST_KVSTRLEN1 = 3; +constexpr char TEST_KVSTR2[] = "baz"; +constexpr uint8_t TEST_KVSTRLEN2 = 3; +constexpr char TEST_KVSTR3[] = "quux"; +constexpr uint8_t TEST_KVSTRLEN3 = 4; +constexpr char TEST_KVSTR4[] = "spam"; +constexpr uint8_t TEST_KVSTRLEN4 = 4; +constexpr char TEST_KVSTR5[] = "xyzzx"; +constexpr uint8_t TEST_KVSTRLEN5 = 5; +constexpr char TEST_KVSTR6[] = "corvid"; +constexpr uint8_t TEST_KVSTRLEN6 = 6; static bool testSetKV(Dictionary &pb, const char *k, uint8_t kl, const char *v, uint8_t vl) { - bool ok; + bool ok; std::cout << "test Set " << k << "->" << v << "\n"; ok = pb.Set(k, kl, v, vl) == 0; std::cout << "\tSet complete\n"; @@ -36,69 +38,70 @@ testSetKV(Dictionary &pb, const char *k, uint8_t kl, const char *v, int main(int argc, const char *argv[]) { - (void)argc; (void)argv; + (void) argc; + (void) argv; - Arena arena; - TLV::Record value; - TLV::Record expect; + Arena arena; + TLV::Record value; + TLV::Record expect; std::cout << "TESTPROG: " << argv[0] << "\n"; - #if defined(__linux__) +#if defined(__linux__) if (arena.Create(ARENA_FILE, ARENA_SIZE) == -1) { abort(); } - #else +#else if (arena.SetAlloc(ARENA_SIZE) == -1) { abort(); } - #endif +#endif std::cout << arena << "\n"; TLV::SetRecord(expect, DICTIONARY_TAG_VAL, TEST_KVSTRLEN3, TEST_KVSTR3); - Dictionary dict(arena); - assert(!dict.Contains(TEST_KVSTR2, TEST_KVSTRLEN2)); + Dictionary dict(arena); + TestAssert(!dict.Contains(TEST_KVSTR2, TEST_KVSTRLEN2)); - assert(testSetKV(dict, TEST_KVSTR1, TEST_KVSTRLEN1, TEST_KVSTR3, - TEST_KVSTRLEN3)); + TestAssert(testSetKV(dict, TEST_KVSTR1, TEST_KVSTRLEN1, TEST_KVSTR3, + TEST_KVSTRLEN3)); std::cout << dict; - assert(testSetKV(dict, TEST_KVSTR2, TEST_KVSTRLEN2, TEST_KVSTR3, - TEST_KVSTRLEN3)); + TestAssert(testSetKV(dict, TEST_KVSTR2, TEST_KVSTRLEN2, TEST_KVSTR3, + TEST_KVSTRLEN3)); std::cout << dict; - assert(dict.Contains(TEST_KVSTR2, TEST_KVSTRLEN2)); - assert(testSetKV(dict, TEST_KVSTR4, TEST_KVSTRLEN4, TEST_KVSTR5, - TEST_KVSTRLEN5)); + TestAssert(dict.Contains(TEST_KVSTR2, TEST_KVSTRLEN2)); + TestAssert(testSetKV(dict, TEST_KVSTR4, TEST_KVSTRLEN4, TEST_KVSTR5, + TEST_KVSTRLEN5)); std::cout << dict; - assert(dict.Lookup(TEST_KVSTR2, TEST_KVSTRLEN2, value)); + TestAssert(dict.Lookup(TEST_KVSTR2, TEST_KVSTRLEN2, value)); - assert(cmpRecord(value, expect)); + TestAssert(cmpRecord(value, expect)); std::cout << "test overwriting key" << "\n"; - assert(testSetKV(dict, TEST_KVSTR2, TEST_KVSTRLEN2, TEST_KVSTR6, - TEST_KVSTRLEN6)); + TestAssert(testSetKV(dict, TEST_KVSTR2, TEST_KVSTRLEN2, TEST_KVSTR6, + TEST_KVSTRLEN6)); std::cout << dict; TLV::SetRecord(expect, DICTIONARY_TAG_VAL, TEST_KVSTRLEN6, TEST_KVSTR6); std::cout << "\tlookup" << "\n"; - assert(dict.Lookup(TEST_KVSTR2, TEST_KVSTRLEN2, value)); + TestAssert(dict.Lookup(TEST_KVSTR2, TEST_KVSTRLEN2, value)); std::cout << "\tcompare records" << "\n"; - assert(cmpRecord(value, expect)); + TestAssert(cmpRecord(value, expect)); std::cout << "\tadd new key to dictionary" << "\n"; - assert(testSetKV(dict, TEST_KVSTR3, TEST_KVSTRLEN3, TEST_KVSTR5, - TEST_KVSTRLEN5)); + TestAssert(testSetKV(dict, TEST_KVSTR3, TEST_KVSTRLEN3, TEST_KVSTR5, + TEST_KVSTRLEN5)); std::cout << dict; TLV::SetRecord(expect, DICTIONARY_TAG_VAL, TEST_KVSTRLEN5, TEST_KVSTR5); - assert(dict.Lookup(TEST_KVSTR4, TEST_KVSTRLEN4, value)); - assert(cmpRecord(value, expect)); + TestAssert(dict.Lookup(TEST_KVSTR4, TEST_KVSTRLEN4, value)); + TestAssert(cmpRecord(value, expect)); - std::cout << "OK" <<"\n"; + std::cout << "OK" << "\n"; // Dump the generated arena for inspection later. - #if defined(__linux__) - #else +#if defined(__linux__) +#else dict.DumpToFile(ARENA_FILE); - #endif +#endif arena.Clear(); std::cout << dict;