From 2b2e82730c1261d7b3cb378869055052ce9d8c34 Mon Sep 17 00:00:00 2001 From: Robin Davies Date: Wed, 16 Mar 2022 00:18:17 -0400 Subject: [PATCH] Automated Build Tests Fixes #34 --- .vscode/launch.json | 2 +- mem_guard,x | 0 src/BeastServerTest.cpp | 41 ++++++---- src/CMakeLists.txt | 8 ++ src/Lv2HostLeakTest.cpp | 25 ++++++- src/MemDebug.cpp | 141 +++++++++++++++++++++++++++-------- src/MemDebug.hpp | 33 ++++++++ src/PiPedalAlsaTest.cpp | 5 +- src/SystemConfigFile.cpp | 8 +- src/SystemConfigFile.hpp | 1 + src/SystemConfigFileTest.cpp | 92 +++++++++++++++++++---- src/json.hpp | 19 ++++- src/jsonTest.cpp | 2 +- 13 files changed, 307 insertions(+), 70 deletions(-) create mode 100644 mem_guard,x create mode 100644 src/MemDebug.hpp diff --git a/.vscode/launch.json b/.vscode/launch.json index c1a7e43..77a7e3e 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -10,7 +10,7 @@ "request": "launch", // Resolved by CMake Tools: "program": "${command:cmake.launchTargetPath}", - "args": [ "[bluetooth_service]" ], + "args": [ "[Dev]" ], "stopAtEntry": false, "cwd": "${workspaceFolder}", "environment": [ diff --git a/mem_guard,x b/mem_guard,x new file mode 100644 index 0000000..e69de29 diff --git a/src/BeastServerTest.cpp b/src/BeastServerTest.cpp index 7135a9c..f6660e1 100644 --- a/src/BeastServerTest.cpp +++ b/src/BeastServerTest.cpp @@ -19,25 +19,38 @@ #include "pch.h" #include "catch.hpp" - #include "BeastServer.hpp" +#include "MemDebug.hpp" +#include using namespace pipedal; -TEST_CASE( "BeastServer shutdown", "[beastServerShutdown]" ) { +TEST_CASE("BeastServer shutdown", "[beastServerShutdown][Build][Dev]") +{ + MemStats initialMemory = GetMemStats(); + { + auto const address = boost::asio::ip::make_address("0.0.0.0"); + auto const port = 8081; + std::string doc_root = "."; + auto const threads = 3; - auto const address = boost::asio::ip::make_address("0.0.0.0"); - auto const port = 8081; - std::string doc_root = "."; - auto const threads = 3; + auto server = createBeastServer( + address, port, doc_root.c_str(), threads); + server->RunInBackground(); + sleep(5); + server->ShutDown(1000); + sleep(1); + server->Join(); + } + MemStats finalMemory = GetMemStats(); - auto server = createBeastServer( - address,port,doc_root.c_str(),threads); - server->RunInBackground(); - sleep(30000); - server->ShutDown(1000); - sleep(1); - server->Join(); + // currently leaking one allocation of 8 bytes. Acceptable. + if (finalMemory.allocations > initialMemory.allocations+1) + { + std::cout << "Leaked Allocations: " << (finalMemory.allocations- initialMemory.allocations) << std::endl; + std::cout << "Leaked Memory: " << (finalMemory.allocated - initialMemory.allocated) << " bytes" << std::endl; + + REQUIRE(finalMemory.allocations == initialMemory.allocations); + } } - diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9aa1264..641bc6c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -189,6 +189,7 @@ add_executable(pipedaltest ${PIPEDAL_SOURCES} testMain.cpp SystemConfigFileTest.cpp BeastServerTest.cpp MemDebug.cpp + MemDebug.hpp ) configure_file(config.hpp.in config.hpp) @@ -207,6 +208,13 @@ target_link_libraries(pipedaltest PRIVATE pthread atomic stdc++fs asound ${LILV_0_LIBRARIES} ${JACK_LIBRARIES} ${LIBNL3_LIBRARIES} systemd SDBusCpp::sdbus-c++ ) + +# Build machine tests. Run tests that are not dependent on hardware. +add_test(NAME BuildTest COMMAND pipedaltest "[Build]") + +# Developer tests. Run tests that only succeed on a Raspberry Pi with attached UBS Audio. +add_test(NAME DevTest COMMAND pipedaltest "[Dev]") + ################################# diff --git a/src/Lv2HostLeakTest.cpp b/src/Lv2HostLeakTest.cpp index dbce0d3..16bdc16 100644 --- a/src/Lv2HostLeakTest.cpp +++ b/src/Lv2HostLeakTest.cpp @@ -25,13 +25,32 @@ #include "Lv2Host.hpp" +#include "MemDebug.hpp" using namespace pipedal; -TEST_CASE( "Lv2Host memory leak", "[lv2host_leak]" ) { +TEST_CASE( "Lv2Host memory leak", "[lv2host_leak][Build][Dev]" ) { - Lv2Host host; + MemStats initialMemory = GetMemStats(); + { + Lv2Host host; - host.Load("/usr/lib/lv2:/usr/local/lib/lv2:/usr/modep/lv2"); + host.Load("/usr/lib/lv2:/usr/local/lib/lv2:/usr/modep/lv2"); + } + MemStats finalMemory = GetMemStats(); + + // Something lilv leaks a Dublin Core url. + // Acceptable. + const int ACCEPTABLE_ALLOCATION_LEAKS = 4; + const int ACCEPTABLE_MEMORY_LEAK = 400; + + if (finalMemory.allocations > initialMemory.allocations + ACCEPTABLE_ALLOCATION_LEAKS + || finalMemory.allocated > initialMemory.allocated + ACCEPTABLE_MEMORY_LEAK) + { + std::cout << "Leaked Allocations: " << (finalMemory.allocations- initialMemory.allocations) << std::endl; + std::cout << "Leaked Memory: " << (finalMemory.allocated - initialMemory.allocated) << " bytes" << std::endl; + + REQUIRE(finalMemory.allocations == initialMemory.allocations); + } } \ No newline at end of file diff --git a/src/MemDebug.cpp b/src/MemDebug.cpp index 9b7dc63..51b9460 100644 --- a/src/MemDebug.cpp +++ b/src/MemDebug.cpp @@ -1,69 +1,148 @@ +// Copyright (c) 2022 Robin Davies +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of +// this software and associated documentation files (the "Software"), to deal in +// the Software without restriction, including without limitation the rights to +// use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of +// the Software, and to permit persons to whom the Software is furnished to do so, +// subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS +// FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR +// COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER +// IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +// CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + #include "stdlib.h" #include -#ifdef DEBUG +#include "MemDebug.hpp" +#include +using namespace pipedal; static std::mutex memMutex; -struct MemStats { - size_t allocated = 0; - size_t allocations = 0; + + +static MemStats memStats; + +const MemStats&pipedal::GetMemStats() +{ + return memStats; +} + +const size_t MEM_GUARD = (size_t)0xBAADF00DBAADF00D; + +struct MemHeader { + size_t size = 0; + MemHeader*pNext = nullptr; + MemHeader *pPrev = nullptr; + size_t mem_guard = MEM_GUARD; + }; -MemStats memStats; +static MemHeader memHead; void *operator new(size_t size) { + char *p = (char*)malloc(size+32+4); { std::lock_guard lock(memMutex); + MemHeader *pHead = (MemHeader*)p; + pHead->size = size; + pHead->pNext = nullptr; + pHead->pPrev = nullptr; + pHead->mem_guard = MEM_GUARD; memStats.allocated += size; memStats.allocations++; - } - char *p = (char*)malloc(size+8); - ((size_t*)p)[0] = size; - return p+8; + // link block into allocation chain + if (memHead.pNext == nullptr) + { + memHead.pNext = memHead.pPrev = pHead; + pHead->pNext = pHead->pPrev = &memHead; + } else { + pHead->pNext = memHead.pNext; + pHead->pNext->pPrev = pHead; + pHead->pPrev = &memHead; + memHead.pNext = pHead; + } + + p = p+32; + char *endGuard = p + size; + *endGuard++ = (char)0xBA; + *endGuard++ = (char)0xAD; + *endGuard++ = (char)0xF0; + *endGuard++ = (char)0x0D; + + + } + return p; } +static void bad_alloc(); + void operator delete(void*mem) { + if (mem == nullptr) return; + char *p = ((char*)mem); - size_t size = *(size_t*)(p-8); { + MemHeader *pHeader = (MemHeader*)(p-32); std::lock_guard lock(memMutex); - memStats.allocated -= size; + memStats.allocated -= pHeader->size; memStats.allocations--; + + // check the memory guards. + if (pHeader->mem_guard != MEM_GUARD) + { + throw std::bad_alloc(); + } + char *pTail = p + pHeader->size; + if (*pTail++ != (char)0xBA) { + throw std::bad_alloc(); + } + if (*pTail++ != (char)0xAD) { + throw std::bad_alloc(); + } + if (*pTail++ != (char)0xF0) { + throw std::bad_alloc(); + } + if (*pTail++ != (char)0x0D) { + throw std::bad_alloc(); + } + // invalidate the memory guards (double free, eg). + pHeader->mem_guard = (size_t)0xDDDDDDDD; + pTail = p + pHeader->size; + *pTail++ = (char)0xDD; + *pTail++ = (char)0xDD; + *pTail++ = (char)0xDD; + *pTail++ = (char)0xDD; + + // unlink the block from the allocation chain. + pHeader->pNext->pPrev = pHeader->pPrev; + pHeader->pPrev->pNext = pHeader->pNext; + pHeader->pNext = nullptr; + pHeader->pPrev = nullptr; + } + free(p-32); - free(p-8); } void* operator new[](size_t size) { - { - std::lock_guard lock(memMutex); - memStats.allocated += size; - memStats.allocations++; - } - - char *p = (char*)malloc(size+8); - ((size_t*)p)[0] = size; - return p+8; + return operator new(size); } void operator delete[](void*mem) { - char *p = ((char*)mem); - size_t size = *(size_t*)(p-8); - { - std::lock_guard lock(memMutex); - memStats.allocated -= size; - memStats.allocations--; - } - - free(p-8); + return operator delete(mem); } -#endif \ No newline at end of file diff --git a/src/MemDebug.hpp b/src/MemDebug.hpp new file mode 100644 index 0000000..54db268 --- /dev/null +++ b/src/MemDebug.hpp @@ -0,0 +1,33 @@ +// Copyright (c) 2022 Robin Davies +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of +// this software and associated documentation files (the "Software"), to deal in +// the Software without restriction, including without limitation the rights to +// use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of +// the Software, and to permit persons to whom the Software is furnished to do so, +// subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS +// FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR +// COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER +// IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +// CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + + +#include + +#pragma once + +namespace pipedal { +struct MemStats { + size_t allocated = 0; + size_t allocations = 0; +}; + +const MemStats &GetMemStats(); + +} diff --git a/src/PiPedalAlsaTest.cpp b/src/PiPedalAlsaTest.cpp index 5dad4af..9316a0c 100644 --- a/src/PiPedalAlsaTest.cpp +++ b/src/PiPedalAlsaTest.cpp @@ -22,16 +22,17 @@ #include #include #include +#include #include "PiPedalAlsa.hpp" using namespace pipedal; -TEST_CASE( "ALSA Test", "[pipedal_alsa_test]" ) { +TEST_CASE( "ALSA Test", "[pipedal_alsa_test][Build][Dev]" ) { PiPedalAlsaDevices devices; auto result = devices.GetAlsaDevices(); - REQUIRE(result.size() >= 1); + std::cout << result.size() << " ALSA devices found." << 0; } diff --git a/src/SystemConfigFile.cpp b/src/SystemConfigFile.cpp index 53e8844..ea0099d 100644 --- a/src/SystemConfigFile.cpp +++ b/src/SystemConfigFile.cpp @@ -44,7 +44,6 @@ static std::string makeLine(const std::string &key, const std::string &value) void SystemConfigFile::Load(const std::filesystem::path &path) { - this->lines.clear(); ifstream f(path); if (!f.is_open()) @@ -53,6 +52,12 @@ void SystemConfigFile::Load(const std::filesystem::path &path) s << "File not found: " << path; throw PiPedalException(s.str()); } + Load(f); + this->currentPath = path; +} +void SystemConfigFile::Load(std::istream&f) { + this->lines.clear(); + while (true) { @@ -64,7 +69,6 @@ void SystemConfigFile::Load(const std::filesystem::path &path) } lines.push_back(line); } - this->currentPath = path; } int64_t SystemConfigFile::GetLine(const std::string &key) const diff --git a/src/SystemConfigFile.hpp b/src/SystemConfigFile.hpp index 0b252e0..e9c9671 100644 --- a/src/SystemConfigFile.hpp +++ b/src/SystemConfigFile.hpp @@ -36,6 +36,7 @@ public: Load(path); } + void Load(std::istream &input); void Load(const std::filesystem::path&path); void Save(std::ostream &os); diff --git a/src/SystemConfigFileTest.cpp b/src/SystemConfigFileTest.cpp index 0d8ea9c..9f86d4b 100644 --- a/src/SystemConfigFileTest.cpp +++ b/src/SystemConfigFileTest.cpp @@ -25,30 +25,94 @@ #include "PiPedalException.hpp" #include - #include "SystemConfigFile.hpp" - using namespace pipedal; +static std::string GetTestFile() +{ + std::stringstream s; + s + << "interface=wlan0" << std::endl + << "driver=nl80211" << std::endl + << "country_code=CA" << std::endl + << "" << std::endl + << "# Wi-Fi features" << std::endl + << "ieee80211n=1" << std::endl + << "ieee80211d=1" << std::endl + << "ieee80211ac=1" << std::endl + << "ht_capab=[HT40][SHORT-GI-20][DSSS_CCK-40]" << std::endl + << "" << std::endl + << "# Authentication options" << std::endl + << "wmm_enabled=1" << std::endl + << "auth_algs=1" << std::endl + << "wpa=2" << std::endl + << "wpa_pairwise=TKIP CCMP" << std::endl + << "wpa_ptk_rekey=600" << std::endl + << "rsn_pairwise=CCMP" << std::endl + << "" << std::endl + << "# Access point configuration" << std::endl + << "ssid=pipedal" << std::endl + << "hw_mode=g" << std::endl + << "channel=8" << std::endl + << "wpa_passphrase=PASSWORD" << std::endl; -TEST_CASE( "SystemConfigFile Test", "[system_config_file_test]" ) { - std::filesystem::path path("/etc/hostapd/hostapd.conf"); + return s.str(); +} +static std::string GetExpectedFile() +{ + std::stringstream s; + s + << "interface=wlan0" << std::endl + << "driver=nl80211" << std::endl + << "country_code=FR" << std::endl + << "" << std::endl + << "# Wi-Fi features" << std::endl + << "ieee80211n=1" << std::endl + << "ieee80211d=1" << std::endl + << "ieee80211ac=1" << std::endl + << "ht_capab=[HT40][SHORT-GI-20][DSSS_CCK-40]" << std::endl + << "" << std::endl + << "# Authentication options" << std::endl + << "wmm_enabled=1" << std::endl + << "auth_algs=1" << std::endl + << "wpa=2" << std::endl + << "wpa_pairwise=TKIP CCMP" << std::endl + << "wpa_ptk_rekey=600" << std::endl + << "rsn_pairwise=CCMP" << std::endl + << "" << std::endl + << "# Access point configuration" << std::endl + << "ssid=newSsid" << std::endl + << "hw_mode=g" << std::endl + << "channel=8" << std::endl + << "wpa_passphrase=PASSWORD" << std::endl + << "" << std::endl + << "# A new feature." << std::endl + << "xx=new_feature" << std::endl; + return s.str(); +} - SystemConfigFile file(path); - std::string driverName; +TEST_CASE("SystemConfigFile Test", "[system_config_file_test]") +{ + + std::stringstream testFile(GetTestFile()); + + SystemConfigFile file; + file.Load(testFile); REQUIRE(file.Get("driver") == "nl80211"); - file.Set("ssid","ssid","Name of the access point"); - file.Set("xx","new_feature","A new feature."); - file.Set("ssid","newSsid"); - file.Set("country_code","FR"); - - - file.Save(std::cout); + file.Set("ssid", "ssid", "Name of the access point"); + file.Set("xx", "new_feature", "A new feature."); + file.Set("ssid", "newSsid"); + file.Set("country_code", "FR"); + std::stringstream outputFile; + file.Save(outputFile); + std::string stringOutput = outputFile.str(); + std::cout << stringOutput; + std::string expected = GetExpectedFile(); + REQUIRE(stringOutput == expected); } - diff --git a/src/json.hpp b/src/json.hpp index aa48af7..65f55f2 100644 --- a/src/json.hpp +++ b/src/json.hpp @@ -162,10 +162,17 @@ private: std::vector* > members_; json_map_impl(const json_map_impl&) { } // disable copy constructor. public: - json_map_impl(std::vector*> members) + using members_t = std::vector*>; + + json_map_impl(const members_t &members) : members_(members) { + } + json_map_impl(members_t &&members) + : members_(std::forward(members)) + { + } virtual ~json_map_impl() { @@ -185,12 +192,20 @@ public: class json_map { public: + template class storage_type : public json_map_impl { public: - storage_type(std::vector*> members) + + using members_t = std::vector*>; + + storage_type(const members_t &members) : json_map_impl(members) { } + storage_type(members_t &&members) + : json_map_impl(std::forward(members)) + { + } }; template static json_member_reference * diff --git a/src/jsonTest.cpp b/src/jsonTest.cpp index 085729e..33ecee5 100644 --- a/src/jsonTest.cpp +++ b/src/jsonTest.cpp @@ -149,7 +149,7 @@ TEST_CASE( "json read", "[json_read_test]" ) { } -TEST_CASE( "json smart ptrs", "[json_smart_ptrs]" ) { +TEST_CASE( "json smart ptrs", "[json_smart_ptrs][Build][Dev]" ) { std::string json =get_json();