Fix potential stale pointer bug in schematic highlight connection code.

SCH_CONNECTION objects are temporary and can become stale any time the
connectivity is updated.  Keeping them around to reference later is a
bad idea.  Even if the object pointer is still valid in an SCH_ITEM in
the undo/redo buffers, comparing the pointer against another pointer as
a test to see if they are the same connection is not valid.  Saving the
connection name is safe and ensures the connection is the same even if
the pointers differ.

(cherry picked from commit 831a6d55fc)
This commit is contained in:
Wayne Stambaugh 2023-05-16 20:06:21 -04:00
parent 0a3f5b65bf
commit 441c2b50b9
8 changed files with 83 additions and 72 deletions

View File

@ -3,7 +3,7 @@
* *
* Copyright (C) 2019 Jean-Pierre Charras, jp.charras at wanadoo.fr * Copyright (C) 2019 Jean-Pierre Charras, jp.charras at wanadoo.fr
* Copyright (C) 2011 Wayne Stambaugh <stambaughw@gmail.com> * Copyright (C) 2011 Wayne Stambaugh <stambaughw@gmail.com>
* Copyright (C) 2004-2022 KiCad Developers, see AUTHORS.txt for contributors. * Copyright (C) 2004-2023 KiCad Developers, see AUTHORS.txt for contributors.
* *
* This program is free software; you can redistribute it and/or * This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License * modify it under the terms of the GNU General Public License
@ -207,9 +207,9 @@ void SCH_EDIT_FRAME::ExecuteRemoteCommand( const char* cmdline )
wxString netName = FROM_UTF8( text ); wxString netName = FROM_UTF8( text );
if( auto sg = Schematic().ConnectionGraph()->FindFirstSubgraphByName( netName ) ) if( auto sg = Schematic().ConnectionGraph()->FindFirstSubgraphByName( netName ) )
m_highlightedConn = sg->m_driver_connection; m_highlightedConn = sg->m_driver_connection->Name();
else else
m_highlightedConn = nullptr; m_highlightedConn = wxEmptyString;
GetToolManager()->RunAction( EE_ACTIONS::updateNetHighlighting, true ); GetToolManager()->RunAction( EE_ACTIONS::updateNetHighlighting, true );

View File

@ -1,7 +1,7 @@
/* /*
* This program source code file is part of KiCad, a free EDA CAD application. * This program source code file is part of KiCad, a free EDA CAD application.
* *
* Copyright (C) 2019-2022 KiCad Developers, see AUTHORS.txt for contributors. * Copyright (C) 2019-2023 KiCad Developers, see AUTHORS.txt for contributors.
* *
* This program is free software; you can redistribute it and/or * This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License * modify it under the terms of the GNU General Public License
@ -202,9 +202,9 @@ bool DIALOG_GLOBAL_EDIT_TEXT_AND_GRAPHICS::TransferDataToWindow()
m_netFilter->SetValue( g_netFilter ); m_netFilter->SetValue( g_netFilter );
m_netFilterOpt->SetValue( true ); m_netFilterOpt->SetValue( true );
} }
else if( m_parent->GetHighlightedConnection() ) else if( !m_parent->GetHighlightedConnection().IsEmpty() )
{ {
m_netFilter->SetValue( m_parent->GetHighlightedConnection()->Name() ); m_netFilter->SetValue( m_parent->GetHighlightedConnection() );
} }
else if( m_selection.GetSize() ) else if( m_selection.GetSize() )
{ {

View File

@ -115,8 +115,7 @@ END_EVENT_TABLE()
SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) : SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
SCH_BASE_FRAME( aKiway, aParent, FRAME_SCH, wxT( "Eeschema" ), wxDefaultPosition, SCH_BASE_FRAME( aKiway, aParent, FRAME_SCH, wxT( "Eeschema" ), wxDefaultPosition,
wxDefaultSize, KICAD_DEFAULT_DRAWFRAME_STYLE, SCH_EDIT_FRAME_NAME ), wxDefaultSize, KICAD_DEFAULT_DRAWFRAME_STYLE, SCH_EDIT_FRAME_NAME )
m_highlightedConn( nullptr )
{ {
m_maximizeByDefault = true; m_maximizeByDefault = true;
m_schematic = new SCHEMATIC( nullptr ); m_schematic = new SCHEMATIC( nullptr );
@ -1442,18 +1441,11 @@ void SCH_EDIT_FRAME::initScreenZoom()
void SCH_EDIT_FRAME::RecalculateConnections( SCH_CLEANUP_FLAGS aCleanupFlags ) void SCH_EDIT_FRAME::RecalculateConnections( SCH_CLEANUP_FLAGS aCleanupFlags )
{ {
const SCH_CONNECTION* highlight = GetHighlightedConnection(); bool highlightedConnChanged = false;
SCH_ITEM* highlightedItem = nullptr; wxString highlightedConn = GetHighlightedConnection();
SCH_SHEET_PATH highlightPath;
if( highlight )
{
highlightPath = highlight->LocalSheet();
highlightedItem = dynamic_cast<SCH_ITEM*>( GetItem( highlight->Parent()->m_Uuid ) );
}
SCHEMATIC_SETTINGS& settings = Schematic().Settings(); SCHEMATIC_SETTINGS& settings = Schematic().Settings();
SCH_SHEET_LIST list = Schematic().GetSheets(); SCH_SHEET_LIST list = Schematic().GetSheets();
#ifdef PROFILE #ifdef PROFILE
PROF_TIMER timer; PROF_TIMER timer;
#endif #endif
@ -1481,6 +1473,11 @@ void SCH_EDIT_FRAME::RecalculateConnections( SCH_CLEANUP_FLAGS aCleanupFlags )
[&]( SCH_ITEM* aChangedItem ) -> void [&]( SCH_ITEM* aChangedItem ) -> void
{ {
GetCanvas()->GetView()->Update( aChangedItem, KIGFX::REPAINT ); GetCanvas()->GetView()->Update( aChangedItem, KIGFX::REPAINT );
SCH_CONNECTION* connection = aChangedItem->Connection();
if( connection && ( connection->Name() == highlightedConn ) )
highlightedConnChanged = true;
}; };
if( !ADVANCED_CFG::GetCfg().m_IncrementalConnectivity || aCleanupFlags == GLOBAL_CLEANUP if( !ADVANCED_CFG::GetCfg().m_IncrementalConnectivity || aCleanupFlags == GLOBAL_CLEANUP
@ -1607,8 +1604,13 @@ void SCH_EDIT_FRAME::RecalculateConnections( SCH_CLEANUP_FLAGS aCleanupFlags )
return flags; return flags;
} ); } );
if( highlightedItem ) if( !highlightedConn.IsEmpty() )
SetHighlightedConnection( highlightedItem->Connection( &highlightPath ) ); {
if( highlightedConnChanged )
wxLogDebug( wxS( "Highlighted connection \"%s\" changed." ), highlightedConn );
else if( !Schematic().ConnectionGraph()->FindFirstSubgraphByName( highlightedConn ) )
wxLogDebug( wxS( "Highlighted connection \"%s\" no longer exists." ), highlightedConn );
}
} }
@ -1712,10 +1714,10 @@ void SCH_EDIT_FRAME::ShowChangedLanguage()
void SCH_EDIT_FRAME::UpdateNetHighlightStatus() void SCH_EDIT_FRAME::UpdateNetHighlightStatus()
{ {
if( const SCH_CONNECTION* conn = GetHighlightedConnection() ) if( !GetHighlightedConnection().IsEmpty() )
{ {
SetStatusText( wxString::Format( _( "Highlighted net: %s" ), SetStatusText( wxString::Format( _( "Highlighted net: %s" ),
UnescapeString( conn->Name() ) ) ); UnescapeString( GetHighlightedConnection() ) ) );
} }
else else
{ {

View File

@ -293,12 +293,12 @@ public:
*/ */
void SendCrossProbeClearHighlight(); void SendCrossProbeClearHighlight();
const SCH_CONNECTION* GetHighlightedConnection() const const wxString& GetHighlightedConnection() const
{ {
return m_highlightedConn; return m_highlightedConn;
} }
void SetHighlightedConnection( const SCH_CONNECTION* aConnection ) void SetHighlightedConnection( const wxString& aConnection )
{ {
m_highlightedConn = aConnection; m_highlightedConn = aConnection;
} }
@ -916,7 +916,7 @@ private:
friend class SCH_FIND_REPLACE_TOOL; friend class SCH_FIND_REPLACE_TOOL;
SCHEMATIC* m_schematic; ///< The currently loaded schematic SCHEMATIC* m_schematic; ///< The currently loaded schematic
const SCH_CONNECTION* m_highlightedConn; ///< The highlighted net or bus, or nullptr wxString m_highlightedConn; ///< The highlighted net or bus or empty string.
wxPageSetupDialogData m_pageSetupData; wxPageSetupDialogData m_pageSetupData;
std::vector<std::unique_ptr<SCH_ITEM>> m_items_to_repeat; ///< For the repeat-last-item cmd std::vector<std::unique_ptr<SCH_ITEM>> m_items_to_repeat; ///< For the repeat-last-item cmd

View File

@ -2,7 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application. * This program source code file is part of KiCad, a free EDA CAD application.
* *
* Copyright (C) 2019 CERN * Copyright (C) 2019 CERN
* Copyright (C) 2019-2022 KiCad Developers, see AUTHORS.txt for contributors. * Copyright (C) 2019-2023 KiCad Developers, see AUTHORS.txt for contributors.
* *
* This program is free software; you can redistribute it and/or * This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License * modify it under the terms of the GNU General Public License
@ -221,7 +221,7 @@ bool EE_SELECTION_TOOL::Init()
{ {
SCH_EDIT_FRAME* editFrame = dynamic_cast<SCH_EDIT_FRAME*>( m_frame ); SCH_EDIT_FRAME* editFrame = dynamic_cast<SCH_EDIT_FRAME*>( m_frame );
return editFrame && editFrame->GetHighlightedConnection() != nullptr; return editFrame && !editFrame->GetHighlightedConnection().IsEmpty();
}; };
auto haveSymbol = auto haveSymbol =

View File

@ -168,7 +168,7 @@ bool SCH_EDIT_TOOL::Init()
{ {
SCH_EDIT_FRAME* editFrame = dynamic_cast<SCH_EDIT_FRAME*>( m_frame ); SCH_EDIT_FRAME* editFrame = dynamic_cast<SCH_EDIT_FRAME*>( m_frame );
return editFrame && editFrame->GetHighlightedConnection() != nullptr; return editFrame && !editFrame->GetHighlightedConnection().IsEmpty();
}; };
auto anyTextTool = auto anyTextTool =

View File

@ -626,9 +626,11 @@ int SCH_EDITOR_CONTROL::SimProbe( const TOOL_EVENT& aEvent )
selectionTool->BrightenItem( m_pickerItem ); selectionTool->BrightenItem( m_pickerItem );
} }
if( m_frame->GetHighlightedConnection() != conn ) wxString connectionName = ( conn ) ? conn->Name() : wxS( "" );
if( m_frame->GetHighlightedConnection() != connectionName )
{ {
m_frame->SetHighlightedConnection( conn ); m_frame->SetHighlightedConnection( connectionName );
TOOL_EVENT dummyEvent; TOOL_EVENT dummyEvent;
UpdateNetHighlighting( dummyEvent ); UpdateNetHighlighting( dummyEvent );
@ -641,9 +643,9 @@ int SCH_EDITOR_CONTROL::SimProbe( const TOOL_EVENT& aEvent )
if( m_pickerItem ) if( m_pickerItem )
m_toolMgr->GetTool<EE_SELECTION_TOOL>()->UnbrightenItem( m_pickerItem ); m_toolMgr->GetTool<EE_SELECTION_TOOL>()->UnbrightenItem( m_pickerItem );
if( m_frame->GetHighlightedConnection() ) if( !m_frame->GetHighlightedConnection().IsEmpty() )
{ {
m_frame->SetHighlightedConnection( nullptr ); m_frame->SetHighlightedConnection( wxEmptyString );
TOOL_EVENT dummyEvent; TOOL_EVENT dummyEvent;
UpdateNetHighlighting( dummyEvent ); UpdateNetHighlighting( dummyEvent );
@ -801,16 +803,18 @@ static bool highlightNet( TOOL_MANAGER* aToolMgr, const VECTOR2D& aPosition )
} }
} }
if( !conn || conn == editFrame->GetHighlightedConnection() ) wxString connectionName = ( conn ) ? conn->Name() : wxS( "" );
if( !conn || connectionName == editFrame->GetHighlightedConnection() )
{ {
editFrame->SetStatusText( wxT( "" ) ); editFrame->SetStatusText( wxT( "" ) );
editFrame->SendCrossProbeClearHighlight(); editFrame->SendCrossProbeClearHighlight();
editFrame->SetHighlightedConnection( nullptr ); editFrame->SetHighlightedConnection( wxEmptyString );
} }
else else
{ {
editFrame->SetCrossProbeConnection( conn ); editFrame->SetCrossProbeConnection( conn );
editFrame->SetHighlightedConnection( conn ); editFrame->SetHighlightedConnection( connectionName );
} }
editFrame->UpdateNetHighlightStatus(); editFrame->UpdateNetHighlightStatus();
@ -1010,29 +1014,21 @@ int SCH_EDITOR_CONTROL::UpdateNetHighlighting( const TOOL_EVENT& aEvent )
SCH_SCREEN* screen = m_frame->GetCurrentSheet().LastScreen(); SCH_SCREEN* screen = m_frame->GetCurrentSheet().LastScreen();
CONNECTION_GRAPH* connectionGraph = m_frame->Schematic().ConnectionGraph(); CONNECTION_GRAPH* connectionGraph = m_frame->Schematic().ConnectionGraph();
std::vector<EDA_ITEM*> itemsToRedraw; std::vector<EDA_ITEM*> itemsToRedraw;
const SCH_CONNECTION* selectedConn = m_frame->GetHighlightedConnection();
if( !screen ) wxCHECK( screen && connectionGraph, 0 );
return 0;
bool selectedIsBus = selectedConn ? selectedConn->IsBus() : false; bool selectedIsBus = false;
wxString selectedName = selectedConn ? selectedConn->Name() : wxString( wxS( "" ) ); wxString selectedName = m_frame->GetHighlightedConnection();
bool selectedIsNoNet = false; bool selectedIsNoNet = false;
CONNECTION_SUBGRAPH* selectedSubgraph = nullptr; CONNECTION_SUBGRAPH* selectedSubgraph = nullptr;
if( selectedConn && selectedConn->Driver() == nullptr )
{
selectedIsNoNet = true;
selectedSubgraph = connectionGraph->GetSubgraphForItem( selectedConn->Parent() );
}
for( SCH_ITEM* item : screen->Items() ) for( SCH_ITEM* item : screen->Items() )
{ {
bool redraw = item->IsBrightened(); bool redraw = item->IsBrightened();
bool highlight = false; bool highlight = false;
if( selectedConn ) if( !selectedName.IsEmpty() )
{ {
SCH_CONNECTION* itemConn = nullptr; SCH_CONNECTION* itemConn = nullptr;
SCH_SYMBOL* symbol = nullptr; SCH_SYMBOL* symbol = nullptr;
@ -1045,9 +1041,19 @@ int SCH_EDITOR_CONTROL::UpdateNetHighlighting( const TOOL_EVENT& aEvent )
else else
itemConn = item->Connection(); itemConn = item->Connection();
if( itemConn && ( selectedName == itemConn->Name() ) )
{
selectedIsBus = itemConn->IsBus();
if( itemConn->Driver() == nullptr )
{
selectedIsNoNet = true;
selectedSubgraph = connectionGraph->GetSubgraphForItem( itemConn->Parent() );
}
if( selectedIsNoNet && selectedSubgraph ) if( selectedIsNoNet && selectedSubgraph )
{ {
for( SCH_ITEM* subgraphItem : selectedSubgraph->m_items ) for( SCH_ITEM* subgraphItem : selectedSubgraph->GetItems() )
{ {
if( item == subgraphItem ) if( item == subgraphItem )
{ {
@ -1058,7 +1064,7 @@ int SCH_EDITOR_CONTROL::UpdateNetHighlighting( const TOOL_EVENT& aEvent )
} }
else if( selectedIsBus && itemConn && itemConn->IsNet() ) else if( selectedIsBus && itemConn && itemConn->IsNet() )
{ {
for( const std::shared_ptr<SCH_CONNECTION>& member : selectedConn->Members() ) for( const std::shared_ptr<SCH_CONNECTION>& member : itemConn->Members() )
{ {
if( member->Name() == itemConn->Name() ) if( member->Name() == itemConn->Name() )
{ {
@ -1067,7 +1073,8 @@ int SCH_EDITOR_CONTROL::UpdateNetHighlighting( const TOOL_EVENT& aEvent )
} }
else if( member->IsBus() ) else if( member->IsBus() )
{ {
for( const std::shared_ptr<SCH_CONNECTION>& bus_member : member->Members() ) for( const std::shared_ptr<SCH_CONNECTION>& bus_member :
member->Members() )
{ {
if( bus_member->Name() == itemConn->Name() ) if( bus_member->Name() == itemConn->Name() )
{ {
@ -1078,11 +1085,13 @@ int SCH_EDITOR_CONTROL::UpdateNetHighlighting( const TOOL_EVENT& aEvent )
} }
} }
} }
else if( selectedConn && itemConn && selectedName == itemConn->Name() ) else if( !selectedName.IsEmpty() && itemConn
&& ( selectedName == itemConn->Name() ) )
{ {
highlight = true; highlight = true;
} }
} }
}
if( highlight ) if( highlight )
item->SetBrightened(); item->SetBrightened();

View File

@ -2,7 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application. * This program source code file is part of KiCad, a free EDA CAD application.
* *
* Copyright (C) 2019 CERN * Copyright (C) 2019 CERN
* Copyright (C) 2019-2022 KiCad Developers, see AUTHORS.txt for contributors. * Copyright (C) 2019-2023 KiCad Developers, see AUTHORS.txt for contributors.
* *
* This program is free software; you can redistribute it and/or * This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License * modify it under the terms of the GNU General Public License
@ -207,7 +207,7 @@ bool SCH_LINE_WIRE_BUS_TOOL::Init()
{ {
SCH_EDIT_FRAME* editFrame = dynamic_cast<SCH_EDIT_FRAME*>( m_frame ); SCH_EDIT_FRAME* editFrame = dynamic_cast<SCH_EDIT_FRAME*>( m_frame );
return editFrame && editFrame->GetHighlightedConnection() != nullptr; return editFrame && !editFrame->GetHighlightedConnection().IsEmpty();
}; };
auto& ctxMenu = m_menu.GetMenu(); auto& ctxMenu = m_menu.GetMenu();