haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix crash when saving pasted root sheet symbols. 

The paste code creates invalid KIID_PATH objects with a zero length.  All
paths must include the root KIID at a minimum.  Code was added to prune
the invalid paths and some defensive code was adding to the formatter to
prevent the crash in case there are other paths where someone generates
a zero length path.

Pruning orphaned instance data does not work when loading schematics prior
to version 7 because the paths did not include the root sheet UUID.  Once
the file is saved to V7 or later, the next load will prune any orphaned or
invalid instance data for the current project.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/16245

(cherry picked from commit 764df1d107)
This commit is contained in:
Wayne Stambaugh 2023-12-03 07:47:57 -05:00
parent 54c325a631
commit 62297e63a2
6 changed files with 40 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
eeschema/files-io.cpp
View File

@ -437,9 +437,6 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector<wxString>& aFileSet, in
// to the s-expression format.
schematic.ReplaceDuplicateTimeStamps();
schematic.PruneOrphanedSymbolInstances( Prj().GetProjectName(),
Schematic().GetSheets() );
// Allow the schematic to be saved to new file format without making any edits.
OnModify();
}
@ -468,6 +465,8 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector<wxString>& aFileSet, in
screen->MigrateSimModels();
}
schematic.PruneOrphanedSymbolInstances( Prj().GetProjectName(), Schematic().GetSheets() );
Schematic().ConnectionGraph()->Reset();
SetScreen( GetCurrentSheet().LastScreen() );

3
eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp
View File

@ -785,6 +785,9 @@ void SCH_SEXPR_PLUGIN::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSchema
for( size_t i = 0; i < aSymbol->GetInstanceReferences().size(); i++ )
{
// Zero length KIID_PATH objects are not valid and will cause a crash below.
wxCHECK2( aSymbol->GetInstanceReferences()[i].m_Path.size(), continue );
// If the instance data is part of this design but no longer has an associated sheet
// path, don't save it. This prevents large amounts of orphaned instance data for the
// current project from accumulating in the schematic files.

30
eeschema/sch_screen.cpp
View File

@ -1630,6 +1630,10 @@ size_t SCH_SCREEN::getLibSymbolNameMatches( const SCH_SYMBOL& aSymbol,
void SCH_SCREEN::PruneOrphanedSymbolInstances( const wxString& aProjectName,
const SCH_SHEET_LIST& aValidSheetPaths )
{
// The project name cannot be empty. Projects older than 7.0 did not save project names
// when saving instance data. Running this algorithm with an empty project name would
// clobber all instance data for projects other than the current one when a schematic
// file is shared across multiple projects.
wxCHECK( !aProjectName.IsEmpty(), /* void */ );
for( SCH_ITEM* item : Items().OfType( SCH_SYMBOL_T ) )
@ -1638,38 +1642,30 @@ void SCH_SCREEN::PruneOrphanedSymbolInstances( const wxString& aProjectName,
wxCHECK2( symbol, continue );
std::set<SCH_SHEET_PATH> pathsToPrune;
std::set<KIID_PATH> pathsToPrune;
const std::vector<SCH_SYMBOL_INSTANCE> instances = symbol->GetInstanceReferences();
for( const SCH_SYMBOL_INSTANCE& instance : instances )
{
// Ignore instance paths from other projects.
if( aProjectName != instance.m_ProjectName )
continue;
std::optional<SCH_SHEET_PATH> pathFound =
aValidSheetPaths.GetSheetPathByKIIDPath( instance.m_Path );
// Check for paths from other projects.
// Check for paths that do not exist in the current project and paths that do
// not contain the current symbol.
if( !pathFound )
{
pathsToPrune.emplace( pathFound.value() );
continue;
}
// Check for paths that are in that are not valid for this screen.
for( const SCH_SHEET_PATH& path : aValidSheetPaths )
{
if( path.LastScreen() == this )
continue;
pathsToPrune.emplace( path );
}
pathsToPrune.emplace( instance.m_Path );
else if( pathFound.value().LastScreen() != this )
pathsToPrune.emplace( pathFound.value().Path() );
}
for( const SCH_SHEET_PATH& sheetPath : pathsToPrune )
for( const KIID_PATH& sheetPath : pathsToPrune )
{
wxLogTrace( traceSchSheetPaths, wxS( "Pruning project '%s' symbol instance %s." ),
aProjectName, sheetPath.Path().AsString() );
aProjectName, sheetPath.AsString() );
symbol->RemoveInstance( sheetPath );
}
}

12
eeschema/sch_screen.h
View File

@ -545,6 +545,18 @@ public:
*/
void MigrateSimModels();
/**
* Remove all invalid symbol instance data in this screen object for the project defined
* by \a aProjectName and the list of \a aValidSheetPaths.
*
* @warning This method will assert and exit on debug builds when \a aProjectName is empty.
*
* @note This method does not affect instance data for any other projects.
*
* @param aProjectName is the name of the current project.
* @param aValidSheetPaths is the list of valid #SCH_SHEET_PATH objects for the current
* project.
*/
void PruneOrphanedSymbolInstances( const wxString& aProjectName,
const SCH_SHEET_LIST& aValidSheetPaths );

3
eeschema/sch_symbol.cpp
View File

@ -641,7 +641,8 @@ void SCH_SYMBOL::RemoveInstance( const KIID_PATH& aInstancePath )
void SCH_SYMBOL::SortInstances( bool (*aSortFunction)( const SCH_SYMBOL_INSTANCE& aLhs,
const SCH_SYMBOL_INSTANCE& aRhs ) )
{
std::sort( m_instanceReferences.begin(), m_instanceReferences.end(), aSortFunction );
if( m_instanceReferences.size() > 1 )
std::sort( m_instanceReferences.begin(), m_instanceReferences.end(), aSortFunction );
}

8
eeschema/tools/sch_editor_control.cpp
View File

@ -1936,6 +1936,14 @@ int SCH_EDITOR_CONTROL::Paste( const TOOL_EVENT& aEvent )
m_frame->GetCurrentSheet().UpdateAllScreenReferences();
// The copy operation creates instance paths that are not valid for the current project or
// saved as part of another project. Prune them now so they do not accumulate in the saved
// schematic file.
SCH_SCREENS allScreens( m_frame->Schematic().Root() );
allScreens.PruneOrphanedSymbolInstances( m_frame->Prj().GetProjectName(),
m_frame->Schematic().GetSheets() );
// Now clear the previous selection, select the pasted items, and fire up the "move" tool.
m_toolMgr->RunAction( EE_ACTIONS::clearSelection, true );
m_toolMgr->RunAction( EE_ACTIONS::addItemsToSel, true, &loadedItems );