Do not iterate over lists you are modifying

The iterator changes when you modify the list, leading to unpredictable
results.  Using the count, working from the back ensures that our list
is stable when removing fields.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/19672
This commit is contained in:
Seth Hillbrand
2025-01-17 17:39:31 -08:00
parent 7972ba84fa
commit c16d403dd0
+13 -29
View File
@@ -381,6 +381,8 @@ void BACK_ANNOTATE::applyChangelist()
{
return b ? _( "true" ) : _( "false" );
};
if( !m_dryRun )
commit.Modify( symbol, screen );
if( m_processReferences && ref.GetRef() != fpData.m_ref && !skip )
{
@@ -390,10 +392,7 @@ void BACK_ANNOTATE::applyChangelist()
fpData.m_ref );
if( !m_dryRun )
{
commit.Modify( symbol, screen );
symbol->SetRef( &ref.GetSheetPath(), fpData.m_ref );
}
m_reporter.ReportHead( msg, RPT_SEVERITY_ACTION );
}
@@ -407,10 +406,7 @@ void BACK_ANNOTATE::applyChangelist()
EscapeHTML( fpData.m_footprint ) );
if( !m_dryRun )
{
commit.Modify( symbol, screen );
symbol->SetFootprintFieldText( fpData.m_footprint );
}
m_reporter.ReportHead( msg, RPT_SEVERITY_ACTION );
}
@@ -424,10 +420,7 @@ void BACK_ANNOTATE::applyChangelist()
EscapeHTML( fpData.m_value ) );
if( !m_dryRun )
{
commit.Modify( symbol, screen );
symbol->SetValueFieldText( fpData.m_value );
}
m_reporter.ReportHead( msg, RPT_SEVERITY_ACTION );
}
@@ -441,10 +434,7 @@ void BACK_ANNOTATE::applyChangelist()
boolString( fpData.m_DNP ) );
if( !m_dryRun )
{
commit.Modify( symbol, screen );
symbol->SetDNP( fpData.m_DNP );
}
m_reporter.ReportHead( msg, RPT_SEVERITY_ACTION );
}
@@ -458,10 +448,7 @@ void BACK_ANNOTATE::applyChangelist()
boolString( fpData.m_excludeFromBOM ) );
if( !m_dryRun )
{
commit.Modify( symbol, screen );
symbol->SetExcludedFromBOM( fpData.m_excludeFromBOM );
}
m_reporter.ReportHead( msg, RPT_SEVERITY_ACTION );
}
@@ -525,10 +512,7 @@ void BACK_ANNOTATE::applyChangelist()
EscapeHTML( fpFieldValue ) );
if( !m_dryRun )
{
commit.Modify( symbol, screen );
symField->SetText( fpFieldValue );
}
m_reporter.ReportHead( msg, RPT_SEVERITY_ACTION );
}
@@ -544,8 +528,6 @@ void BACK_ANNOTATE::applyChangelist()
if( !m_dryRun )
{
commit.Modify( symbol, screen );
SCH_FIELD newField( VECTOR2I( 0, 0 ), symbol->GetFieldCount(), symbol,
fpFieldName );
newField.SetText( fpFieldValue );
@@ -558,26 +540,28 @@ void BACK_ANNOTATE::applyChangelist()
// 3. Existing field has been deleted from footprint and needs to be deleted from symbol
// Check all symbol fields for existence in the footprint field map
for( SCH_FIELD& field : symbol->GetFields() )
for( int ii = symbol->GetFieldCount() - 1; ii >= 0; --ii )
{
// Never delete mandatory fields
if( field.IsMandatory() )
SCH_FIELD* field = symbol->GetFieldById( ii );
if( !field )
continue;
if( fpData.m_fieldsMap.find( field.GetCanonicalName() )
// Never delete mandatory fields
if( field->IsMandatory() )
continue;
if( fpData.m_fieldsMap.find( field->GetCanonicalName() )
== fpData.m_fieldsMap.end() )
{
// Field not found in footprint field map, delete it
m_changesCount++;
msg.Printf( _( "Delete %s field '%s.'" ),
ref.GetRef(),
EscapeHTML( field.GetCanonicalName() ) );
EscapeHTML( field->GetCanonicalName() ) );
if( !m_dryRun )
{
commit.Modify( symbol, screen );
symbol->RemoveField( &field );
}
symbol->RemoveField( field );
m_reporter.ReportHead( msg, RPT_SEVERITY_ACTION );
}