From 70f14277d193a8627877d851ae1065668492fdd5 Mon Sep 17 00:00:00 2001 From: Roberto Fernandez Bautista Date: Sun, 28 Jan 2024 20:07:44 +0100 Subject: [PATCH] PNS Debug/regresions: Fix some memory leaks --- qa/tools/pns/pns_log_file.cpp | 21 ++++++------ qa/tools/pns/pns_log_file.h | 31 +++++++++-------- qa/tools/pns/pns_test_debug_decorator.cpp | 5 ++- qa/tools/pns/qa_pns_regressions_main.cpp | 42 +++++++++++++++-------- 4 files changed, 59 insertions(+), 40 deletions(-) diff --git a/qa/tools/pns/pns_log_file.cpp b/qa/tools/pns/pns_log_file.cpp index a768c3e5ea..1056ee27dd 100644 --- a/qa/tools/pns/pns_log_file.cpp +++ b/qa/tools/pns/pns_log_file.cpp @@ -69,7 +69,8 @@ PNS_LOG_FILE::PNS_LOG_FILE() : m_routerSettings.reset( new PNS::ROUTING_SETTINGS( nullptr, "" ) ); } -std::shared_ptr parseShape( SHAPE_TYPE expectedType, wxStringTokenizer& aTokens ) + +std::shared_ptr PNS_LOG_FILE::parseShape( SHAPE_TYPE expectedType, wxStringTokenizer& aTokens ) { SHAPE_TYPE type = static_cast ( wxAtoi( aTokens.GetNextToken() ) ); @@ -119,15 +120,15 @@ bool PNS_LOG_FILE::parseCommonPnsProps( PNS::ITEM* aItem, const wxString& cmd, return false; } -PNS::SEGMENT* PNS_LOG_FILE::parsePnsSegmentFromString( wxStringTokenizer& aTokens ) +std::unique_ptr PNS_LOG_FILE::parsePnsSegmentFromString( wxStringTokenizer& aTokens ) { - PNS::SEGMENT* seg = new PNS::SEGMENT(); + std::unique_ptr seg( new PNS::SEGMENT() ); while( aTokens.CountTokens() ) { wxString cmd = aTokens.GetNextToken(); - if( !parseCommonPnsProps( seg, cmd, aTokens ) ) + if( !parseCommonPnsProps( seg.get(), cmd, aTokens ) ) { if( cmd == wxS( "shape" ) ) { @@ -145,15 +146,15 @@ PNS::SEGMENT* PNS_LOG_FILE::parsePnsSegmentFromString( wxStringTokenizer& aToken return seg; } -PNS::VIA* PNS_LOG_FILE::parsePnsViaFromString( wxStringTokenizer& aTokens ) +std::unique_ptr PNS_LOG_FILE::parsePnsViaFromString( wxStringTokenizer& aTokens ) { - PNS::VIA* via = new PNS::VIA(); + std::unique_ptr via( new PNS::VIA() ); while( aTokens.CountTokens() ) { wxString cmd = aTokens.GetNextToken(); - if( !parseCommonPnsProps( via, cmd, aTokens ) ) + if( !parseCommonPnsProps( via.get(), cmd, aTokens ) ) { if( cmd == wxS( "shape" ) ) { @@ -178,7 +179,7 @@ PNS::VIA* PNS_LOG_FILE::parsePnsViaFromString( wxStringTokenizer& aTokens ) } -PNS::ITEM* PNS_LOG_FILE::parseItemFromString( wxStringTokenizer& aTokens ) +std::unique_ptr PNS_LOG_FILE::parseItemFromString( wxStringTokenizer& aTokens ) { wxString type = aTokens.GetNextToken(); @@ -409,8 +410,8 @@ bool PNS_LOG_FILE::Load( const wxFileName& logFileName, REPORTER* aRpt ) } else if ( cmd == wxT( "added" ) ) { - PNS::ITEM* item = parseItemFromString( tokens ); - m_commitState.m_addedItems.push_back( item ); + m_parsed_items.push_back( parseItemFromString( tokens ) ); + m_commitState.m_addedItems.push_back( m_parsed_items.back().get() ); } else if ( cmd == wxT( "removed" ) ) { diff --git a/qa/tools/pns/pns_log_file.h b/qa/tools/pns/pns_log_file.h index 58a0f772f3..43ecfcdad0 100644 --- a/qa/tools/pns/pns_log_file.h +++ b/qa/tools/pns/pns_log_file.h @@ -55,16 +55,14 @@ public: struct COMMIT_STATE { - COMMIT_STATE() {}; + COMMIT_STATE(){}; COMMIT_STATE( const COMMIT_STATE& aOther ) : - m_removedIds( aOther.m_removedIds ), - m_addedItems( aOther.m_addedItems ) + m_removedIds( aOther.m_removedIds ), m_addedItems( aOther.m_addedItems ) { - } - std::set m_removedIds; - std::vector m_addedItems; + std::set m_removedIds; + std::vector m_addedItems; bool Compare( const COMMIT_STATE& aOther ); }; @@ -91,19 +89,22 @@ public: private: bool parseCommonPnsProps( PNS::ITEM* aItem, const wxString& cmd, wxStringTokenizer& aTokens ); - PNS::SEGMENT* parsePnsSegmentFromString( wxStringTokenizer& aTokens ); + std::unique_ptr parsePnsSegmentFromString( wxStringTokenizer& aTokens ); + + std::unique_ptr parsePnsViaFromString( wxStringTokenizer& aTokens ); - PNS::VIA* parsePnsViaFromString( wxStringTokenizer& aTokens ); + std::unique_ptr parseItemFromString( wxStringTokenizer& aTokens ); - PNS::ITEM* parseItemFromString( wxStringTokenizer& aTokens ); + std::shared_ptr parseShape( SHAPE_TYPE expectedType, wxStringTokenizer& aTokens ); private: - std::shared_ptr m_settingsMgr; - std::unique_ptr m_routerSettings; - std::vector m_events; - std::shared_ptr m_board; - COMMIT_STATE m_commitState; - PNS::ROUTER_MODE m_mode; + std::shared_ptr m_settingsMgr; + std::unique_ptr m_routerSettings; + std::vector m_events; + std::shared_ptr m_board; + COMMIT_STATE m_commitState; + std::vector> m_parsed_items; + PNS::ROUTER_MODE m_mode; }; #endif diff --git a/qa/tools/pns/pns_test_debug_decorator.cpp b/qa/tools/pns/pns_test_debug_decorator.cpp index 07626a8a10..9787e27c8c 100644 --- a/qa/tools/pns/pns_test_debug_decorator.cpp +++ b/qa/tools/pns/pns_test_debug_decorator.cpp @@ -108,6 +108,7 @@ PNS_DEBUG_STAGE::PNS_DEBUG_STAGE() PNS_DEBUG_STAGE::~PNS_DEBUG_STAGE() { + delete m_entries; } @@ -123,6 +124,8 @@ PNS_TEST_DEBUG_DECORATOR::PNS_TEST_DEBUG_DECORATOR( REPORTER* aReporter ) : PNS_TEST_DEBUG_DECORATOR::~PNS_TEST_DEBUG_DECORATOR() { + for( PNS_DEBUG_STAGE* stage : m_stages ) + delete stage; // fixme: I know it's a hacky tool but it should clean after itself at some point... } @@ -261,7 +264,7 @@ void PNS_TEST_DEBUG_DECORATOR::NewStage( const wxString& name, int iter, stage->m_name = name; stage->m_iter = iter; - m_stages.push_back( new PNS_DEBUG_STAGE ); + m_stages.push_back( stage ); m_activeEntry = m_stages.back()->m_entries; } diff --git a/qa/tools/pns/qa_pns_regressions_main.cpp b/qa/tools/pns/qa_pns_regressions_main.cpp index a392919d3f..168e76026f 100644 --- a/qa/tools/pns/qa_pns_regressions_main.cpp +++ b/qa/tools/pns/qa_pns_regressions_main.cpp @@ -75,7 +75,7 @@ bool runSingleTest( REPORTER* aReporter, wxString name, wxString testDirPath ) int main( int argc, char* argv[] ) { - + int passed = 0, failed = 0; @@ -108,6 +108,26 @@ struct PNS_TEST_CASE }; +class FIXTURE_LOGGER +{ +public: + FIXTURE_LOGGER() + { + m_Log = new KI_TEST::CONSOLE_LOG; + m_Reporter = new KI_TEST::CONSOLE_MSG_REPORTER( m_Log ); + } + + ~FIXTURE_LOGGER() + { + delete m_Reporter; + delete m_Log; + } + + KI_TEST::CONSOLE_LOG* m_Log; + KI_TEST::CONSOLE_MSG_REPORTER* m_Reporter; +}; + + class PNS_TEST_FIXTURE { public: @@ -118,20 +138,14 @@ public: PNS_LOG_FILE logFile; PNS_LOG_PLAYER player; - if( !m_log ) - m_log = new KI_TEST::CONSOLE_LOG; - - if( !m_reporter ) - m_reporter = new KI_TEST::CONSOLE_MSG_REPORTER( m_log ); - - if( !logFile.Load( wxString( aTestData->GetDataPath() ), m_reporter ) ) + if( !logFile.Load( wxString( aTestData->GetDataPath() ), m_logger.m_Reporter ) ) { - m_reporter->Report( wxString::Format( "Failed to load test '%s' from '%s'", + m_logger.m_Reporter->Report( wxString::Format( "Failed to load test '%s' from '%s'", aTestData->GetName(), aTestData->GetDataPath() ), RPT_SEVERITY_ERROR ); } - player.SetReporter( m_reporter ); + player.SetReporter( m_logger.m_Reporter ); player.ReplayLog( &logFile, 0 ); bool pass = player.CompareResults( &logFile ); @@ -139,12 +153,12 @@ public: BOOST_TEST_FAIL( "replay results inconsistent with reference reslts" ); } - static KI_TEST::CONSOLE_LOG* m_log; - static KI_TEST::CONSOLE_MSG_REPORTER* m_reporter; + static FIXTURE_LOGGER m_logger; }; -KI_TEST::CONSOLE_LOG* PNS_TEST_FIXTURE::m_log = nullptr; -KI_TEST::CONSOLE_MSG_REPORTER* PNS_TEST_FIXTURE::m_reporter = nullptr; + +FIXTURE_LOGGER PNS_TEST_FIXTURE::m_logger; + std::vector createTestCases() {