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

Don't rely on live pointers in search data. 

Try as you might to keep them fresh, there's no gaurantee.
Instead, just treat them as weak references and search the
existing items for a pointer-value match.

This also allows us to remove some code which kept recycling
the search position back to the start ever time a replace
was done.

Fixes: lp:1559258
* https://bugs.launchpad.net/kicad/+bug/1559258
This commit is contained in:
Jeff Young 2018-02-12 01:31:11 +00:00 committed by Wayne Stambaugh
parent 2a9aa11422
commit 9d26253b2c
3 changed files with 84 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
eeschema/find.cpp
View File

@ -316,20 +316,6 @@ void SCH_EDIT_FRAME::OnFindSchematicItem( wxFindDialogEvent& aEvent )
{
if( m_foundItems.GetCount() == 0 )
return;
// Refresh the search cache in case something has changed. This prevents any stale
// pointers from crashing Eeschema when the wxEVT_FIND_CLOSE event is handled.
if( IsSearchCacheObsolete( searchCriteria ) )
{
if( aEvent.GetFlags() & FR_CURRENT_SHEET_ONLY && g_RootSheet->CountSheets() > 1 )
{
m_foundItems.Collect( searchCriteria, m_CurrentSheet );
}
else
{
m_foundItems.Collect( searchCriteria );
}
}
}
else if( IsSearchCacheObsolete( searchCriteria ) )
{
@ -358,7 +344,6 @@ void SCH_EDIT_FRAME::OnFindSchematicItem( wxFindDialogEvent& aEvent )
void SCH_EDIT_FRAME::OnFindReplace( wxFindDialogEvent& aEvent )
{
static int nextFoundIndex = 0;
SCH_ITEM* item;
SCH_SHEET_PATH* sheet;
SCH_SHEET_LIST schematic( g_RootSheet );
@ -380,10 +365,6 @@ void SCH_EDIT_FRAME::OnFindReplace( wxFindDialogEvent& aEvent )
{
m_foundItems.Collect( searchCriteria );
}
// Restore the next found index on cache refresh. Prevents single replace events
// from starting back at the beginning of the cache.
m_foundItems.SetFoundIndex( nextFoundIndex );
}
if( aEvent.GetEventType() == wxEVT_COMMAND_FIND_REPLACE_ALL )
@ -437,10 +418,12 @@ void SCH_EDIT_FRAME::OnFindReplace( wxFindDialogEvent& aEvent )
OnModify();
SaveUndoItemInUndoList( undoItem );
updateFindReplaceView( aEvent );
// A single replace is part of the search; it does not dirty it.
m_foundItems.SetForceSearch( false );
}
m_foundItems.IncrementIndex();
nextFoundIndex = m_foundItems.GetFoundIndex();
}
// End the replace if we are at the end if the list. This prevents an infinite loop if

84
eeschema/sch_collectors.cpp
View File

@ -333,6 +333,65 @@ bool SCH_COLLECTOR::IsDraggableJunction() const
}
/**
* A singleton item of this class is returned for a weak reference that no longer exists.
* Its sole purpose is to flag the item as having been deleted.
*/
class DELETED_SCH_ITEM : public SCH_ITEM
{
public:
DELETED_SCH_ITEM() :
SCH_ITEM( nullptr, NOT_USED )
{}
wxString GetSelectMenuText() const override { return _( "(Deleted Item)" ); }
wxString GetClass() const override { return wxT( "DELETED_SCH_ITEM" ); }
// define pure virtuals:
wxPoint GetPosition() const override { return wxPoint(); }
void SetPosition( const wxPoint& ) override {}
void Draw( EDA_DRAW_PANEL* , wxDC* , const wxPoint& , GR_DRAWMODE , COLOR4D ) override {}
void Show( int , std::ostream& ) const override {}
void Move( const wxPoint& ) override {}
void MirrorY( int ) override {}
void MirrorX( int ) override {}
void Rotate( wxPoint ) override {}
};
DELETED_SCH_ITEM g_DeletedSchItem;
SCH_ITEM* SCH_FIND_COLLECTOR::GetItem( int ndx ) const
{
if( (unsigned)ndx >= (unsigned)GetCount() )
return NULL;
// Do not simply return m_List[ ndx ] as it might have been deleted. Instead
// treat it as a weak reference and search the sheets for an item with the same
// pointer value.
void* weakRef = m_List[ ndx ];
for( unsigned i = 0; i < m_sheetPaths.size(); i++ )
{
for( EDA_ITEM* item = m_sheetPaths[ i ].LastDrawList(); item; item = item->Next() )
{
if( (void*) item == weakRef )
return (SCH_ITEM*) item;
}
}
return &g_DeletedSchItem;
}
SCH_ITEM* SCH_FIND_COLLECTOR::operator[]( int ndx ) const
{
return GetItem( ndx );
}
bool SCH_FIND_COLLECTOR::PassedEnd() const
{
bool retv = false;
@ -415,9 +474,7 @@ wxString SCH_FIND_COLLECTOR::GetText()
wxT( "Cannot get found item at invalid index." ) );
SCH_FIND_COLLECTOR_DATA data = m_data[ m_foundIndex ];
EDA_ITEM* foundItem = m_List[ m_foundIndex ];
wxCHECK_MSG( foundItem != NULL, wxEmptyString, wxT( "Invalid found item pointer." ) );
EDA_ITEM* foundItem = GetItem( m_foundIndex );
wxString msg;
@ -445,7 +502,7 @@ EDA_ITEM* SCH_FIND_COLLECTOR::GetItem( SCH_FIND_COLLECTOR_DATA& aData )
return NULL;
aData = m_data[ m_foundIndex ];
return m_List[ m_foundIndex ];
return GetItem( m_foundIndex );
}
@ -457,13 +514,10 @@ bool SCH_FIND_COLLECTOR::ReplaceItem( SCH_SHEET_PATH* aSheetPath )
wxCHECK_MSG( IsValidIndex( m_foundIndex ), false,
wxT( "Invalid replace list index in SCH_FIND_COLLECTOR." ) );
EDA_ITEM* item = m_List[ m_foundIndex ];
EDA_ITEM* item = GetItem( m_foundIndex );
bool replaced = item->Replace( m_findReplaceData, aSheetPath );
if( replaced )
SetForceSearch();
return replaced;
}
@ -472,7 +526,7 @@ SEARCH_RESULT SCH_FIND_COLLECTOR::Inspect( EDA_ITEM* aItem, void* aTestData )
{
wxPoint position;
if( aItem->Matches( m_findReplaceData, m_sheetPath, &position ) )
if( aItem->Matches( m_findReplaceData, m_currentSheetPath, &position ) )
{
if( aItem->Type() == LIB_PIN_T )
{
@ -489,7 +543,8 @@ SEARCH_RESULT SCH_FIND_COLLECTOR::Inspect( EDA_ITEM* aItem, void* aTestData )
}
Append( aItem );
m_data.push_back( SCH_FIND_COLLECTOR_DATA( position, m_sheetPath->PathHumanReadable(),
m_data.push_back( SCH_FIND_COLLECTOR_DATA( position,
m_currentSheetPath->PathHumanReadable(),
(SCH_ITEM*) aTestData ) );
}
@ -513,11 +568,13 @@ void SCH_FIND_COLLECTOR::Collect( SCH_FIND_REPLACE_DATA& aFindReplaceData,
Empty(); // empty the collection just in case
m_data.clear();
m_foundIndex = 0;
m_sheetPaths.clear();
SetForceSearch( false );
if( aSheetPath )
{
m_sheetPath = aSheetPath;
m_currentSheetPath = aSheetPath;
m_sheetPaths.push_back( *m_currentSheetPath );
EDA_ITEM::IterateForward( aSheetPath->LastDrawList(), m_inspector, NULL, m_ScanTypes );
}
else
@ -526,8 +583,9 @@ void SCH_FIND_COLLECTOR::Collect( SCH_FIND_REPLACE_DATA& aFindReplaceData,
for( unsigned i = 0; i < schematic.size(); i++ )
{
m_sheetPath = &schematic[i];
EDA_ITEM::IterateForward( m_sheetPath->LastDrawList(), m_inspector, NULL, m_ScanTypes );
m_currentSheetPath = &schematic[i];
m_sheetPaths.push_back( *m_currentSheetPath );
EDA_ITEM::IterateForward( m_currentSheetPath->LastDrawList(), m_inspector, NULL, m_ScanTypes );
}
}

13
eeschema/sch_collectors.h
View File

@ -32,6 +32,7 @@
#include <collector.h>
#include <sch_item_struct.h>
#include <sch_sheet_path.h>
#include <dialogs/dialog_schematic_find.h>
@ -236,8 +237,11 @@ class SCH_FIND_COLLECTOR : public COLLECTOR
/// The criteria used to test for matching items.
SCH_FIND_REPLACE_DATA m_findReplaceData;
/// The path of the sheet currently being iterated over.
SCH_SHEET_PATH* m_sheetPath;
/// The path of the sheet *currently* being iterated over.
SCH_SHEET_PATH* m_currentSheetPath;
/// The paths of the all the sheets in the collector.
SCH_SHEET_PATHS m_sheetPaths;
/// The current found item list index.
int m_foundIndex;
@ -268,7 +272,7 @@ public:
SetScanTypes( aScanTypes );
m_foundIndex = 0;
SetForceSearch( false );
m_sheetPath = NULL;
m_currentSheetPath = NULL;
m_lib_hash = 0;
}
@ -279,6 +283,9 @@ public:
m_data.clear();
}
SCH_ITEM* GetItem( int ndx ) const;
SCH_ITEM* operator[]( int ndx ) const;
void SetForceSearch( bool doSearch = true ) { m_forceSearch = doSearch; }
int GetLibHash() const { return m_lib_hash; }