From e0633ea9ce05e7e57dc483ab81ac462d2dde442e Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Mon, 5 Jan 2026 18:36:57 -0800 Subject: [PATCH] Fix memory leak in FOOTPRINT We had some copy/pasta in the points group and lost dangling pointers from a move operation. Potentially a cause of heap corruption Also set the parser to skip null fields on load Fixes https://gitlab.com/kicad/code/kicad/-/issues/22623 --- pcbnew/footprint.cpp | 22 +++++++++++++++++-- .../pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp | 3 +++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/pcbnew/footprint.cpp b/pcbnew/footprint.cpp index 466ec47356..857ae4f946 100644 --- a/pcbnew/footprint.cpp +++ b/pcbnew/footprint.cpp @@ -887,7 +887,7 @@ FOOTPRINT& FOOTPRINT::operator=( FOOTPRINT&& aOther ) for( PCB_POINT* point : m_points ) delete point; - m_groups.clear(); + m_points.clear(); for( PCB_POINT* point : aOther.Points() ) Add( point ); @@ -948,6 +948,9 @@ FOOTPRINT& FOOTPRINT::operator=( const FOOTPRINT& aOther ) std::map ptrMap; // Copy fields + for( PCB_FIELD* field : m_fields ) + delete field; + m_fields.clear(); for( PCB_FIELD* field : aOther.m_fields ) @@ -958,6 +961,9 @@ FOOTPRINT& FOOTPRINT::operator=( const FOOTPRINT& aOther ) } // Copy pads + for( PAD* pad : m_pads ) + delete pad; + m_pads.clear(); for( PAD* pad : aOther.Pads() ) @@ -968,6 +974,9 @@ FOOTPRINT& FOOTPRINT::operator=( const FOOTPRINT& aOther ) } // Copy zones + for( ZONE* zone : m_zones ) + delete zone; + m_zones.clear(); for( ZONE* zone : aOther.Zones() ) @@ -984,6 +993,9 @@ FOOTPRINT& FOOTPRINT::operator=( const FOOTPRINT& aOther ) } // Copy drawings + for( BOARD_ITEM* item : m_drawings ) + delete item; + m_drawings.clear(); for( BOARD_ITEM* item : aOther.GraphicalItems() ) @@ -994,6 +1006,9 @@ FOOTPRINT& FOOTPRINT::operator=( const FOOTPRINT& aOther ) } // Copy groups + for( PCB_GROUP* group : m_groups ) + delete group; + m_groups.clear(); for( PCB_GROUP* group : aOther.Groups() ) @@ -1007,7 +1022,10 @@ FOOTPRINT& FOOTPRINT::operator=( const FOOTPRINT& aOther ) Add( newGroup ); } - // Copy drawings + // Copy points + for( PCB_POINT* point : m_points ) + delete point; + m_points.clear(); for( PCB_POINT* point : aOther.Points() ) diff --git a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp index 3e3ab4265d..29a28f281a 100644 --- a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp +++ b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp @@ -1225,6 +1225,9 @@ void PCB_IO_KICAD_SEXPR::format( const FOOTPRINT* aFootprint ) const for( const PCB_FIELD* field : aFootprint->GetFields() ) { + if( !field ) + continue; + m_out->Print( "(property %s %s", m_out->Quotew( field->GetCanonicalName() ).c_str(), m_out->Quotew( field->GetText() ).c_str() );