From 6078bc52eb3e9c0aff53613ceffbb128a6156ba9 Mon Sep 17 00:00:00 2001 From: Roberto Fernandez Bautista Date: Sun, 31 Mar 2024 15:09:04 +0200 Subject: [PATCH] PNS: Fix a few memory leaks --- pcbnew/router/pns_kicad_iface.cpp | 5 ++--- pcbnew/router/pns_kicad_iface.h | 4 ++-- pcbnew/router/pns_optimizer.cpp | 25 ++++++++++--------------- pcbnew/router/pns_optimizer.h | 5 ++--- qa/tests/pcbnew/test_pns_basics.cpp | 2 +- qa/tools/pns/pns_log_file.cpp | 7 +++---- qa/tools/pns/pns_log_player.cpp | 23 ++++++++++++++--------- qa/tools/pns/pns_log_player.h | 27 +++++++++++++++++++-------- qa/tools/pns/pns_log_viewer_frame.cpp | 8 ++++---- 9 files changed, 57 insertions(+), 49 deletions(-) diff --git a/pcbnew/router/pns_kicad_iface.cpp b/pcbnew/router/pns_kicad_iface.cpp index 1a824d7f1f..81f126f56a 100644 --- a/pcbnew/router/pns_kicad_iface.cpp +++ b/pcbnew/router/pns_kicad_iface.cpp @@ -1057,14 +1057,13 @@ PNS_KICAD_IFACE::PNS_KICAD_IFACE() PNS_KICAD_IFACE_BASE::~PNS_KICAD_IFACE_BASE() { + delete m_ruleResolver; + delete m_debugDecorator; } PNS_KICAD_IFACE::~PNS_KICAD_IFACE() { - delete m_ruleResolver; - delete m_debugDecorator; - if( m_previewItems ) { m_previewItems->FreeItems(); diff --git a/pcbnew/router/pns_kicad_iface.h b/pcbnew/router/pns_kicad_iface.h index 8ee481d995..36d1d44bf9 100644 --- a/pcbnew/router/pns_kicad_iface.h +++ b/pcbnew/router/pns_kicad_iface.h @@ -52,7 +52,7 @@ class PNS_KICAD_IFACE_BASE : public PNS::ROUTER_IFACE { public: PNS_KICAD_IFACE_BASE(); - ~PNS_KICAD_IFACE_BASE(); + ~PNS_KICAD_IFACE_BASE() override; void EraseView() override {}; void SetBoard( BOARD* aBoard ); @@ -115,7 +115,7 @@ class PNS_KICAD_IFACE : public PNS_KICAD_IFACE_BASE { public: PNS_KICAD_IFACE(); - ~PNS_KICAD_IFACE(); + ~PNS_KICAD_IFACE() override; virtual void SetHostTool( PCB_TOOL_BASE* aTool ); diff --git a/pcbnew/router/pns_optimizer.cpp b/pcbnew/router/pns_optimizer.cpp index f83b42799a..19931d55a4 100644 --- a/pcbnew/router/pns_optimizer.cpp +++ b/pcbnew/router/pns_optimizer.cpp @@ -120,6 +120,10 @@ OPTIMIZER::OPTIMIZER( NODE* aWorld ) : OPTIMIZER::~OPTIMIZER() { + for( OPT_CONSTRAINT* c : m_constraints ) + delete c; + + m_constraints.clear(); } @@ -417,16 +421,7 @@ bool OPTIMIZER::checkColliding( ITEM* aItem, bool aUpdateCache ) } -void OPTIMIZER::ClearConstraints() -{ - for( OPT_CONSTRAINT* c : m_constraints ) - delete c; - - m_constraints.clear(); -} - - -void OPTIMIZER::AddConstraint ( OPT_CONSTRAINT *aConstraint ) +void OPTIMIZER::addConstraint ( OPT_CONSTRAINT *aConstraint ) { m_constraints.push_back( aConstraint ); } @@ -633,20 +628,20 @@ bool OPTIMIZER::Optimize( LINE* aLine, LINE* aResult, LINE* aRoot ) wxString::Format( "opt limit-corner-count root %d maxc %d mask %x", rootObtuseCorners, aLine->SegmentCount(), angleMask ) ); - AddConstraint( c ); + addConstraint( c ); } if( m_effortLevel & PRESERVE_VERTEX ) { auto c = new PRESERVE_VERTEX_CONSTRAINT( m_world, m_preservedVertex ); - AddConstraint( c ); + addConstraint( c ); } if( m_effortLevel & RESTRICT_VERTEX_RANGE ) { auto c = new RESTRICT_VERTEX_RANGE_CONSTRAINT( m_world, m_restrictedVertexRange.first, m_restrictedVertexRange.second ); - AddConstraint( c ); + addConstraint( c ); } if( m_effortLevel & RESTRICT_AREA ) @@ -654,13 +649,13 @@ bool OPTIMIZER::Optimize( LINE* aLine, LINE* aResult, LINE* aRoot ) auto c = new AREA_CONSTRAINT( m_world, m_restrictArea, m_restrictAreaIsStrict ); SHAPE_RECT r( m_restrictArea ); PNS_DBG( dbg, AddShape, &r, YELLOW, 0, wxT( "area-constraint" ) ); - AddConstraint( c ); + addConstraint( c ); } if( m_effortLevel & KEEP_TOPOLOGY ) { auto c = new KEEP_TOPOLOGY_CONSTRAINT( m_world ); - AddConstraint( c ); + addConstraint( c ); } // TODO: Fix for arcs diff --git a/pcbnew/router/pns_optimizer.h b/pcbnew/router/pns_optimizer.h index a05ceeab56..6ff7514913 100644 --- a/pcbnew/router/pns_optimizer.h +++ b/pcbnew/router/pns_optimizer.h @@ -153,9 +153,6 @@ public: m_restrictAreaIsStrict = aStrict; } - void ClearConstraints(); - void AddConstraint ( OPT_CONSTRAINT *aConstraint ); - private: static const int MaxCachedItems = 256; @@ -169,6 +166,8 @@ private: bool m_isStatic; }; + + void addConstraint ( OPT_CONSTRAINT *aConstraint ); bool mergeObtuse( LINE* aLine ); bool mergeFull( LINE* aLine ); bool mergeColinear( LINE* aLine ); diff --git a/qa/tests/pcbnew/test_pns_basics.cpp b/qa/tests/pcbnew/test_pns_basics.cpp index 40802406d0..20b71b79cc 100644 --- a/qa/tests/pcbnew/test_pns_basics.cpp +++ b/qa/tests/pcbnew/test_pns_basics.cpp @@ -288,7 +288,7 @@ public: m_testFixture( aFixture ) {} - ~MOCK_PNS_KICAD_IFACE() {} + ~MOCK_PNS_KICAD_IFACE() override {} void HideItem( PNS::ITEM* aItem ) override {}; void DisplayItem( const PNS::ITEM* aItem, int aClearance, bool aEdit = false, diff --git a/qa/tools/pns/pns_log_file.cpp b/qa/tools/pns/pns_log_file.cpp index abff45a781..133719303f 100644 --- a/qa/tools/pns/pns_log_file.cpp +++ b/qa/tools/pns/pns_log_file.cpp @@ -356,7 +356,6 @@ bool PNS_LOG_FILE::Load( const wxFileName& logFileName, REPORTER* aRpt ) std::shared_ptr drcEngine( new DRC_ENGINE ); - CONSOLE_LOG consoleLog; BOARD_DESIGN_SETTINGS& bds = m_board->GetDesignSettings(); bds.m_DRCEngine = drcEngine; @@ -366,7 +365,7 @@ bool PNS_LOG_FILE::Load( const wxFileName& logFileName, REPORTER* aRpt ) drcEngine->SetBoard( m_board.get() ); drcEngine->SetDesignSettings( &bds ); - drcEngine->SetLogReporter( new CONSOLE_MSG_REPORTER( &consoleLog ) ); + drcEngine->SetLogReporter( aRpt ); drcEngine->InitEngine( wxFileName() ); } catch( const PARSE_ERROR& parse_error ) @@ -405,11 +404,11 @@ bool PNS_LOG_FILE::Load( const wxFileName& logFileName, REPORTER* aRpt ) } else if( cmd == wxT( "event" ) ) { - m_events.push_back( PNS::LOGGER::ParseEvent( line ) ); + m_events.push_back( std::move( PNS::LOGGER::ParseEvent( line ) ) ); } else if ( cmd == wxT( "added" ) ) { - m_parsed_items.push_back( parseItemFromString( tokens ) ); + m_parsed_items.push_back( std::move( 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_player.cpp b/qa/tools/pns/pns_log_player.cpp index 337473e97e..2d61fd14cc 100644 --- a/qa/tools/pns/pns_log_player.cpp +++ b/qa/tools/pns/pns_log_player.cpp @@ -39,8 +39,6 @@ PNS_LOG_PLAYER::PNS_LOG_PLAYER() PNS_LOG_PLAYER::~PNS_LOG_PLAYER() { - if( m_debugDecorator ) - delete m_debugDecorator; } void PNS_LOG_PLAYER::createRouter() @@ -52,7 +50,9 @@ void PNS_LOG_PLAYER::createRouter() m_router->SetInterface( m_iface.get() ); m_router->ClearWorld(); m_router->SyncWorld(); - m_router->LoadSettings( new PNS::ROUTING_SETTINGS( nullptr, "" ) ); + + m_routingSettings.reset( new PNS::ROUTING_SETTINGS( nullptr, "" ) ); + m_router->LoadSettings( m_routingSettings.get() ); m_router->Settings().SetMode( PNS::RM_Walkaround ); m_router->Sizes().SetTrackWidth( 250000 ); @@ -83,6 +83,9 @@ const PNS_LOG_FILE::COMMIT_STATE PNS_LOG_PLAYER::GetRouterUpdatedItems() } // fixme: update the state with the head trace (not supported in current testsuite) + // Note: we own the head items (cloned inside GetUpdatedItems) - we need to delete them! + for( auto head : heads ) + delete head; return state; } @@ -351,17 +354,19 @@ void PNS_LOG_VIEW_TRACKER::SetStage( int aStage ) void PNS_LOG_VIEW_TRACKER::HideItem( PNS::ITEM* aItem ) { ENTRY ent; - ent.isHideOp = true; - ent.item = aItem; - m_vitems[m_currentStage].push_back( ent ); + ent.m_isHideOp = true; + ent.m_item = aItem; + ent.m_ownedItem = nullptr; // I don't own it! + m_vitems[m_currentStage].push_back( std::move( ent ) ); } void PNS_LOG_VIEW_TRACKER::DisplayItem( const PNS::ITEM* aItem ) { ENTRY ent; - ent.isHideOp = false; - ent.item = aItem->Clone(); - m_vitems[m_currentStage].push_back( ent ); + ent.m_isHideOp = false; + ent.m_item = aItem->Clone(); + ent.m_ownedItem.reset( ent.m_item ); //delete me when ENTRY is deleted + m_vitems[m_currentStage].push_back( std::move( ent ) ); //printf("DBG disp cur %d cnt %d\n", m_currentStage, m_vitems[m_currentStage].size() ); } diff --git a/qa/tools/pns/pns_log_player.h b/qa/tools/pns/pns_log_player.h index cde04bfb96..ffeceae775 100644 --- a/qa/tools/pns/pns_log_player.h +++ b/qa/tools/pns/pns_log_player.h @@ -41,8 +41,18 @@ class PNS_LOG_VIEW_TRACKER public: struct ENTRY { - bool isHideOp; - const PNS::ITEM* item; + ENTRY() = default; + ENTRY( const PNS_LOG_VIEW_TRACKER::ENTRY& ) = delete; + + ENTRY( ENTRY&& aOther ) : + m_isHideOp( aOther.m_isHideOp ), + m_item( aOther.m_item ), + m_ownedItem( std::move( aOther.m_ownedItem ) ) + {} + + bool m_isHideOp{ false }; + const PNS::ITEM* m_item{ nullptr }; + std::unique_ptr m_ownedItem{ nullptr }; }; typedef std::vector VIEW_ENTRIES; @@ -66,7 +76,7 @@ class PNS_LOG_PLAYER_KICAD_IFACE : public PNS_KICAD_IFACE_BASE { public: PNS_LOG_PLAYER_KICAD_IFACE( PNS_LOG_VIEW_TRACKER* aViewTracker ); - ~PNS_LOG_PLAYER_KICAD_IFACE(); + ~PNS_LOG_PLAYER_KICAD_IFACE() override; void HideItem( PNS::ITEM* aItem ) override; void DisplayItem( const PNS::ITEM* aItem, int aClearance, bool aEdit = false, @@ -102,11 +112,12 @@ public: private: void createRouter(); - std::shared_ptr m_viewTracker; - PNS_TEST_DEBUG_DECORATOR* m_debugDecorator; - std::shared_ptr m_board; - std::unique_ptr m_iface; - std::unique_ptr m_router; + std::shared_ptr m_viewTracker; + std::unique_ptr m_iface; // needs to be deleted after m_router + PNS_TEST_DEBUG_DECORATOR* m_debugDecorator; + std::shared_ptr m_board; + std::unique_ptr m_router; + std::unique_ptr m_routingSettings; uint64_t m_timeLimitUs; REPORTER* m_reporter; }; diff --git a/qa/tools/pns/pns_log_viewer_frame.cpp b/qa/tools/pns/pns_log_viewer_frame.cpp index 1cc258d20e..eab4786890 100644 --- a/qa/tools/pns/pns_log_viewer_frame.cpp +++ b/qa/tools/pns/pns_log_viewer_frame.cpp @@ -907,10 +907,10 @@ void PNS_LOG_VIEWER_FRAME::updatePnsPreviewItems( int iter ) for( auto& ent : entries ) { - if ( ent.isHideOp ) + if ( ent.m_isHideOp ) { - auto parent = ent.item->Parent(); + auto parent = ent.m_item->Parent(); if( parent ) { @@ -919,8 +919,8 @@ void PNS_LOG_VIEWER_FRAME::updatePnsPreviewItems( int iter ) } else { - ROUTER_PREVIEW_ITEM* pitem = new ROUTER_PREVIEW_ITEM( ent.item, view ); - pitem->Update( ent.item ); + ROUTER_PREVIEW_ITEM* pitem = new ROUTER_PREVIEW_ITEM( ent.m_item, view ); + pitem->Update( ent.m_item ); m_previewItems->Add(pitem); // printf("DBG vadd %p total %d\n", pitem, m_previewItems->GetSize() ); }