From dc21a605317a2b7e41d1e58da492f27a7e4e8133 Mon Sep 17 00:00:00 2001
From: jean-pierre charras <jp.charras@wanadoo.fr>
Date: Wed, 24 Oct 2018 16:18:19 +0200
Subject: [PATCH] Fix incorrect management of SCH_SHEET_PIN items when adding
 or removing them. Especially new SCH_SHEET_PIN items were added twice, thus
 creating crashes.

Fixes: lp:1799606
https://bugs.launchpad.net/kicad/+bug/1799606
---
 eeschema/operations_on_items_lists.cpp |  5 ++---
 eeschema/sch_edit_frame.cpp            | 12 ++++++++----
 eeschema/sch_screen.cpp                |  3 +--
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/eeschema/operations_on_items_lists.cpp b/eeschema/operations_on_items_lists.cpp
index a9a2251925..3cde0120b5 100644
--- a/eeschema/operations_on_items_lists.cpp
+++ b/eeschema/operations_on_items_lists.cpp
@@ -189,9 +189,8 @@ void SCH_EDIT_FRAME::DeleteItem( SCH_ITEM* aItem, bool aAppend )
 
     if( aItem->Type() == SCH_SHEET_PIN_T )
     {
-        RemoveFromScreen( aItem );
-
-        // This item is attached to a node, and is not accessible by the global list directly.
+        // This item is attached to its parent hierarchical sheet,
+        // and is not accessible by the global list directly and cannot be removed from this list.
         SCH_SHEET* sheet = (SCH_SHEET*) aItem->GetParent();
         wxCHECK_RET( (sheet != NULL) && (sheet->Type() == SCH_SHEET_T),
                      wxT( "Sheet label has invalid parent item." ) );
diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp
index d58bf992eb..2681dd3e87 100644
--- a/eeschema/sch_edit_frame.cpp
+++ b/eeschema/sch_edit_frame.cpp
@@ -645,6 +645,7 @@ void SCH_EDIT_FRAME::OnCloseWindow( wxCloseEvent& aEvent )
             return;
 
         viewlibFrame = (LIB_VIEW_FRAME*) Kiway().Player( FRAME_SCH_VIEWER_MODAL, false );
+
         if( viewlibFrame && !viewlibFrame->Close() )   // Can close modal component viewer?
             return;
     }
@@ -1398,13 +1399,16 @@ void SCH_EDIT_FRAME::addCurrentItemToScreen()
             SetSheetNumberAndCount();
         }
 
-        if( !screen->CheckIfOnDrawList( item ) )  // don't want a loop!
-            AddToScreen( item );
+        if(  item->Type() != SCH_SHEET_PIN_T )  // for SCH_SHEET_PIN_T: will be added later
+                                                // to the SCH_SHEET parent
+        {
+            if( !screen->CheckIfOnDrawList( item ) )  // don't want a loop!
+                AddToScreen( item );
+        }
 
         if( undoItem == item )
         {
             SetRepeatItem( item );
-
             SaveCopyInUndoList( undoItem, UR_NEW );
         }
         else
@@ -1414,7 +1418,7 @@ void SCH_EDIT_FRAME::addCurrentItemToScreen()
             // currently: sheet pin or component field.
             // currently, only a sheet pin can be found as new item,
             // because new component fields have a specific handling, and do not appears here
-            SaveCopyInUndoList( undoItem, UR_CHANGED );
+            SaveCopyInUndoList( undoItem, UR_CHANGED );     // Save the parent sheet
 
             if( item->Type() == SCH_SHEET_PIN_T )
                 ( (SCH_SHEET*)undoItem )->AddPin( (SCH_SHEET_PIN*) item );
diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp
index f57d3d151f..9c97617244 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -185,8 +185,6 @@ void SCH_SCREEN::DeleteItem( SCH_ITEM* aItem )
 
     SetModify();
 
-    m_drawList.Remove( aItem );
-
     if( aItem->Type() == SCH_SHEET_PIN_T )
     {
         // This structure is attached to a sheet, get the parent sheet object.
@@ -198,6 +196,7 @@ void SCH_SCREEN::DeleteItem( SCH_ITEM* aItem )
     }
     else
     {
+        m_drawList.Remove( aItem );
         delete aItem;
     }
 }