Recleaning DLIST from pcbnew

This was re-introduced by 5d3e6e3d44

The crash happened b/c we have to manage list containers in each element
and minor adjustments cause the rest of the list to be lost.  This
commit re-implements it using std::iterators and deque

Fixes #2623 | https://gitlab.com/kicad/code/kicad/issues/2623

include/class_board_item.h

@@ -212,16 +212,9 @@ public:
         // only MODULEs & TRACKs can be locked at this time.
     }
 
-    /**
-     * Function UnLink
-     * detaches this object from its owner.  This base class implementation
-     * should work for all derived classes which are held in a DLIST<>.
-     */
-    virtual void UnLink();
-
     /**
      * Function DeleteStructure
-     * deletes this object after UnLink()ing it from its owner if it has one.
+     * deletes this object after removing from its parent if it has one.
      */
     void DeleteStructure();
 

pcbnew/board_connected_item.h

@@ -230,17 +230,6 @@ public:
     {
     }
 
-    void UnLink()
-    {
-        /* if it were needed:
-        DHEAD* list = GetList();
-
-        wxASSERT( list );
-
-        list->remove( this );
-        */
-    }
-
     //-----</ satisfy some virtual functions >-----------------------------
 
     /**

pcbnew/class_board_item.cpp

@@ -51,16 +51,6 @@ wxString BOARD_ITEM::ShowShape( STROKE_T aShape )
 }
 
 
-void BOARD_ITEM::UnLink()
-{
-    DLIST<BOARD_ITEM>* list = (DLIST<BOARD_ITEM>*) GetList();
-    wxASSERT( list );
-
-    if( list )
-        list->Remove( this );
-}
-
-
 BOARD* BOARD_ITEM::GetBoard() const
 {
     if( Type() == PCB_T )

pcbnew/class_module.cpp

@@ -274,7 +274,7 @@ void MODULE::Add( BOARD_ITEM* aBoardItem, ADD_MODE aMode )
     switch( aBoardItem->Type() )
     {
     case PCB_MODULE_TEXT_T:
-        // Only user texts can be added this way. Reference and value are not hold in the DLIST.
+        // Only user text can be added this way.
         assert( static_cast<TEXTE_MODULE*>( aBoardItem )->GetType() == TEXTE_MODULE::TEXT_is_DIVERS );
 
         // no break
@@ -321,7 +321,7 @@ void MODULE::Remove( BOARD_ITEM* aBoardItem )
     switch( aBoardItem->Type() )
     {
     case PCB_MODULE_TEXT_T:
-        // Only user texts can be removed this way. Reference and value are not hold in the DLIST.
+        // Only user text can be removed this way.
         wxCHECK_RET(
                 static_cast<TEXTE_MODULE*>( aBoardItem )->GetType() == TEXTE_MODULE::TEXT_is_DIVERS,
                 "Please report this bug: Invalid remove operation on required text" );

pcbnew/dialogs/dialog_find.cpp

@@ -51,12 +51,11 @@ bool FindIncludeTexts = true;
 bool FindIncludeValues = true;
 bool FindIncludeReferences = true;
 bool FindIncludeMarkers = true;
-//bool findIncludeVias = false;
+
 
 DIALOG_FIND::DIALOG_FIND( PCB_BASE_FRAME* aFrame ) : DIALOG_FIND_BASE( aFrame )
 {
     m_frame = aFrame;
-    m_foundItem = NULL;
     GetSizer()->SetSizeHints( this );
 
     m_searchCombo->Append( m_frame->GetFindHistoryList() );
@@ -84,10 +83,10 @@ DIALOG_FIND::DIALOG_FIND( PCB_BASE_FRAME* aFrame ) : DIALOG_FIND_BASE( aFrame )
     m_includeMarkers->SetValue( FindIncludeMarkers );
 
     m_status->SetLabel( wxEmptyString);
-    m_hitList = new DLIST<BOARD_ITEM>;
+    m_upToDate = false;
-    m_hitList->SetOwnership( false );
+
-    m_itemCount = 0;
+    m_hitList.clear();
-    isUpToDate = false;
+    m_it = m_hitList.begin();
 
     m_findNext->SetDefault();
     SetInitialFocus( m_searchCombo );
@@ -112,7 +111,7 @@ void DIALOG_FIND::onFindPreviousClick( wxCommandEvent& aEvent )
 
 void DIALOG_FIND::onSearchAgainClick( wxCommandEvent& aEvent )
 {
-    isUpToDate = false;
+    m_upToDate = false;
     search( true );
 }
 
@@ -132,7 +131,7 @@ void DIALOG_FIND::search( bool aDirection )
     {
         m_searchCombo->Insert( searchString, 0 );
         m_searchCombo->SetSelection( 0 );
-        isUpToDate = false;
+        m_upToDate = false;
         m_frame->GetFindHistoryList().Insert( searchString, 0 );
 
         if( m_searchCombo->GetCount() > 10 )
@@ -146,7 +145,7 @@ void DIALOG_FIND::search( bool aDirection )
         m_searchCombo->Delete( index );
         m_searchCombo->Insert( searchString, 0 );
         m_searchCombo->SetSelection( 0 );
-        isUpToDate = false;
+        m_upToDate = false;
 
         if( m_frame->GetFindHistoryList().Index( searchString ) )
             m_frame->GetFindHistoryList().Remove( searchString );
@@ -160,19 +159,19 @@ void DIALOG_FIND::search( bool aDirection )
     if( FindOptionCase != m_matchCase->GetValue() )
     {
         FindOptionCase = m_matchCase->GetValue();
-        isUpToDate = false;
+        m_upToDate = false;
     }
 
     if( FindOptionWords != m_matchWords->GetValue() )
     {
         FindOptionWords = m_matchWords->GetValue();
-        isUpToDate = false;
+        m_upToDate = false;
     }
 
     if( FindOptionWildcards != m_wildcards->GetValue() )
     {
         FindOptionWildcards = m_wildcards->GetValue();
-        isUpToDate = false;
+        m_upToDate = false;
     }
 
     FindOptionWrap = m_wrap->GetValue();
@@ -180,25 +179,25 @@ void DIALOG_FIND::search( bool aDirection )
     if( FindIncludeTexts != m_includeTexts->GetValue() )
     {
         FindIncludeTexts = m_includeTexts->GetValue();
-        isUpToDate = false;
+        m_upToDate = false;
     }
 
     if( FindIncludeValues != m_includeValues->GetValue() )
     {
         FindIncludeValues = m_includeValues->GetValue();
-        isUpToDate = false;
+        m_upToDate = false;
     }
 
     if( FindIncludeReferences != m_includeReferences->GetValue() )
     {
         FindIncludeReferences = m_includeReferences->GetValue();
-        isUpToDate = false;
+        m_upToDate = false;
     }
 
     if( FindIncludeMarkers != m_includeMarkers->GetValue() )
     {
         FindIncludeMarkers = m_includeMarkers->GetValue();
-        isUpToDate = false;
+        m_upToDate = false;
     }
 
     if( FindOptionCase )
@@ -218,14 +217,10 @@ void DIALOG_FIND::search( bool aDirection )
     m_frame->GetCanvas()->GetViewStart( &screen->m_StartVisu.x, &screen->m_StartVisu.y );
 
     // Refresh the list of results
-    if( !isUpToDate )
+    if( !m_upToDate )
     {
         m_status->SetLabel( _( "Searching..." ) );
-
+        m_hitList.clear();
-        while( m_hitList->GetCount() > 0 )
-            m_hitList->PopBack();
-
-        m_foundItem = NULL;
 
         if( FindIncludeTexts || FindIncludeValues || FindIncludeReferences )
         {
@@ -236,7 +231,7 @@ void DIALOG_FIND::search( bool aDirection )
                         || ( module->Value().Matches( m_frame->GetFindReplaceData(), nullptr )
                                    && FindIncludeValues ) )
                 {
-                    m_hitList->Append( module );
+                    m_hitList.push_back( module );
                 }
 
                 if( m_includeTexts->GetValue() )
@@ -248,7 +243,7 @@ void DIALOG_FIND::search( bool aDirection )
                         if( textItem
                                 && textItem->Matches( m_frame->GetFindReplaceData(), nullptr ) )
                         {
-                            m_hitList->Append( module );
+                            m_hitList.push_back( module );
                         }
                     }
                 }
@@ -262,7 +257,7 @@ void DIALOG_FIND::search( bool aDirection )
 
                     if( textItem && textItem->Matches( m_frame->GetFindReplaceData(), nullptr ) )
                     {
-                        m_hitList->Append( textItem );
+                        m_hitList.push_back( textItem );
                     }
                 }
             }
@@ -275,38 +270,50 @@ void DIALOG_FIND::search( bool aDirection )
                 MARKER_PCB* marker = m_frame->GetBoard()->GetMARKER( i );
 
                 if( marker->Matches( m_frame->GetFindReplaceData(), nullptr ) )
-                    m_hitList->Append( marker );
+                    m_hitList.push_back( marker );
             }
         }
 
-        m_itemCount = -1;
+        m_upToDate = true;
+
+        if( aDirection )
+            m_it = m_hitList.begin();
+        else
+            m_it = m_hitList.end();
     }
 
     // Do we want a sorting algorithm ? If so, implement it here.
 
     // Get the item to display
-    if( m_hitList->begin() == NULL )
+    if( m_hitList.empty() )
     {
         m_frame->SetStatusText( wxEmptyString );
-        m_itemCount = 0;
-        m_foundItem = NULL;
-    }
-    else if( m_itemCount == -1 )
-    {
-        m_foundItem = aDirection ? m_hitList->begin() : m_hitList->end();
-        m_itemCount = aDirection ? 0 : (int) m_hitList->GetCount() - 1;
-        isUpToDate = true;
     }
     else
     {
         if( aDirection )
         {
-            if( m_itemCount >= static_cast<int>( m_hitList->GetCount() - 1 ) )
+            m_it++;
+
+            if( m_it == m_hitList.end() )
+            {
+                if( m_wrap->GetValue() )
+                    m_it = m_hitList.begin();
+                else
+                {
+                    m_frame->SetStatusText( wxEmptyString );
+                    DisplayError( this, _( "No more item to show" ), 10 );
+                    return;
+                }
+            }
+        }
+        else
+        {
+            if( m_it == m_hitList.begin() )
             {
                 if( m_wrap->GetValue() )
                 {
-                    m_itemCount = 0;
+                    m_it = m_hitList.end();
-                    m_foundItem = m_hitList->begin();
                 }
                 else
                 {
@@ -315,46 +322,22 @@ void DIALOG_FIND::search( bool aDirection )
                     return;
                 }
             }
-            else
+
-            {
+            m_it--;
-                m_itemCount++;
-                m_foundItem = dynamic_cast<BOARD_ITEM*>( m_foundItem->Next() );
-            }
-        }
-        else
-        {
-            if( m_itemCount <= 0 )
-            {
-                if( m_wrap->GetValue() )
-                {
-                    m_itemCount = (int) m_hitList->GetCount() - 1;
-                    m_foundItem = m_hitList->end();
-                }
-                else
-                {
-                    m_frame->SetStatusText( wxEmptyString );
-                    DisplayError( this, _( "No more item to show" ), 10 );
-                    return;
-                }
-            }
-            else
-            {
-                m_itemCount--;
-                m_foundItem = dynamic_cast<BOARD_ITEM*>( m_foundItem->Back() );
-            }
         }
     }
 
     // Display the item
-    if( m_foundItem )
+    if( m_it != m_hitList.end() )
     {
-        m_frame->GetToolManager()->RunAction( PCB_ACTIONS::selectItem, true, m_foundItem );
+        m_frame->GetToolManager()->RunAction( PCB_ACTIONS::selectItem, true, *m_it );
-        m_frame->FocusOnLocation( m_foundItem->GetPosition(), true );
+        m_frame->FocusOnLocation( ( *m_it )->GetPosition(), true );
 
         msg.Printf( _( "\"%s\" found" ), searchString );
         m_frame->SetStatusText( msg );
 
-        msg.Printf( _( "Hit(s): %i / %i" ), m_itemCount + 1, m_hitList->GetCount() );
+        msg.Printf( _( "Hit(s): %ld / %lu" ), std::distance( m_hitList.begin(), m_it ),
+                m_hitList.size() );
         m_status->SetLabel( msg );
     }
     else
@@ -364,12 +347,11 @@ void DIALOG_FIND::search( bool aDirection )
         msg.Printf( _( "\"%s\" not found" ), searchString );
         DisplayError( this, msg, 10 );
 
-        m_itemCount = 0;
         m_status->SetLabel( _( "No hits" ) );
     }
 
     if( m_highlightCallback )
-        m_highlightCallback( m_foundItem );
+        m_highlightCallback( GetItem() );
 }
 
 void DIALOG_FIND::onClose( wxCommandEvent& aEvent )
@@ -384,9 +366,5 @@ void DIALOG_FIND::onClose( wxCommandEvent& aEvent )
     FindIncludeMarkers = m_includeMarkers->GetValue();
     FindIncludeReferences = m_includeReferences->GetValue();
 
-    while( m_hitList->GetCount() > 0 )
-        m_hitList->PopBack();
-
-    delete m_hitList;
     EndModal( 1 );
 }

pcbnew/dialogs/dialog_find.h

@@ -3,7 +3,7 @@
  *
  * Copyright (C) 2012 Marco Mattila <marcom99@gmail.com>
  * Copyright (C) 2006 Jean-Pierre Charras <jean-pierre.charras@gipsa-lab.inpg.fr>
- * Copyright (C) 1992-2012 Kicad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 1992-2019 Kicad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -27,10 +27,13 @@
 #define DIALOG_FIND_BASE_H
 
 #include <boost/function.hpp>
+#include <sys/types.h>
+#include <wx/event.h>
+#include <deque>
+
+#include <class_board_item.h>
+
 #include <dialog_find_base.h>
-#include <dlist.h>
-#include <iterator>
-#include <vector>
 
 using namespace std;
 
@@ -39,11 +42,23 @@ class DIALOG_FIND : public DIALOG_FIND_BASE
 public:
     DIALOG_FIND( PCB_BASE_FRAME* aParent );
 
+    /**
+     * Returns the currently found item or nullptr in the case of no items found
+     * @return
+     */
     inline BOARD_ITEM* GetItem() const
     {
-        return m_foundItem;
+        if( m_it != m_hitList.end() )
+            return *m_it;
+        else
+            return nullptr;
     }
 
+    /**
+     * Function to be called on each found event.  Must be able to handle nullptr in the
+     * case where no item is found
+     * @param aCallback
+     */
     void SetCallback( boost::function<void( BOARD_ITEM* )> aCallback )
     {
         m_highlightCallback = aCallback;
@@ -52,10 +67,9 @@ public:
 
 private:
     PCB_BASE_FRAME*                     m_frame;
-    int                m_itemCount;
+    std::deque<BOARD_ITEM*>             m_hitList;
-    BOARD_ITEM*        m_foundItem;
+    std::deque<BOARD_ITEM*>::iterator   m_it;
-    DLIST<BOARD_ITEM>* m_hitList;
+    bool                                m_upToDate;
-    bool               isUpToDate;
 
     boost::function<void( BOARD_ITEM* )> m_highlightCallback;
 

qa/qa_utils/mocks.cpp

@@ -150,10 +150,9 @@ DIALOG_FIND::DIALOG_FIND( PCB_BASE_FRAME* aParent ) : DIALOG_FIND_BASE( aParent
 {
     // these members are initialized to avoid warnings about non initialized vars
     m_frame = aParent;
-    m_itemCount = 0;
+    m_hitList.clear();
-    m_foundItem = nullptr;
+    m_it = m_hitList.begin();
-    m_hitList = nullptr;
+    m_upToDate = false;
-    isUpToDate = false;
 }
 
 void DIALOG_FIND::onFindNextClick( wxCommandEvent& aEvent )