From 0f1a11ef388a35c05fef4029fd38a94df0655e27 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@ucdavis.edu>
Date: Fri, 11 Jan 2019 17:27:29 -0800
Subject: [PATCH] pcbnew: Cut only copied objects

Fixes a bug where objects where accessed after being freed by the cut

Fixes: lp:1811456
* https://bugs.launchpad.net/kicad/+bug/1811456
---
 common/view/view.cpp       |  1 +
 include/tool/actions.h     |  2 +-
 include/view/view_item.h   |  5 +++++
 pcbnew/tools/edit_tool.cpp | 30 +++++++++++++++++++++++++-----
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/common/view/view.cpp b/common/view/view.cpp
index e8f280949c..fe581b3a96 100644
--- a/common/view/view.cpp
+++ b/common/view/view.cpp
@@ -279,6 +279,7 @@ void VIEW::OnDestroy( VIEW_ITEM* aItem )
         data->m_view->VIEW::Remove( aItem );
 
     delete data;
+    aItem->ClearViewPrivData();
 }
 
 
diff --git a/include/tool/actions.h b/include/tool/actions.h
index 5e7da3eef0..4e9c0564ba 100644
--- a/include/tool/actions.h
+++ b/include/tool/actions.h
@@ -104,7 +104,7 @@ public:
                              CURSOR_CLICK, CURSOR_DBL_CLICK, CURSOR_FAST_MOVE = 0x8000 };
 
     ///> Remove event modifier flags
-    enum class REMOVE_FLAGS { NORMAL = 0x00, ALT = 0x01 };
+    enum class REMOVE_FLAGS { NORMAL = 0x00, ALT = 0x01, CUT = 0x02 };
 };
 
 #endif
diff --git a/include/view/view_item.h b/include/view/view_item.h
index 67d6dad743..854395c88d 100644
--- a/include/view/view_item.h
+++ b/include/view/view_item.h
@@ -150,6 +150,11 @@ public:
         return m_viewPrivData;
     }
 
+    void ClearViewPrivData()
+    {
+        m_viewPrivData = NULL;
+    }
+
 private:
     friend class VIEW;
 
diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index 69b156b7e4..21cb0a8880 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -882,12 +882,19 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
     std::vector<BOARD_ITEM*> lockedItems;
 
     // get a copy instead of reference (as we're going to clear the selection before removing items)
-    auto selectionCopy = m_selectionTool->RequestSelection(
+    SELECTION selectionCopy;
+    bool isCut = aEvent.Parameter<intptr_t>() == static_cast<intptr_t>( PCB_ACTIONS::REMOVE_FLAGS::CUT );
+    bool isAlt = aEvent.Parameter<intptr_t>() == static_cast<intptr_t>( PCB_ACTIONS::REMOVE_FLAGS::ALT );
+
+    // If we are in a "Cut" operation, then the copied selection exists already
+    if( isCut )
+        selectionCopy = m_selectionTool->GetSelection();
+    else
+        selectionCopy = m_selectionTool->RequestSelection(
             []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector )
             { EditToolSelectionFilter( aCollector, EXCLUDE_LOCKED_PADS | EXCLUDE_TRANSIENTS ); } );
 
     bool isHover = selectionCopy.IsHover();
-    const bool isAlt = aEvent.Parameter<intptr_t>() == (int) PCB_ACTIONS::REMOVE_FLAGS::ALT;
 
     // in "alternative" mode, deletion is not just a simple list of selected items,
     // it removes whole tracks, not just segments
@@ -900,7 +907,10 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
     if( selectionCopy.Empty() )
         return 0;
 
-    if( !m_lockedSelected )
+    // N.B. Setting the CUT flag prevents lock filtering as we only want to delete the items that
+    // were copied to the clipboard, no more, no fewer.  Filtering for locked item, if any will be done
+    // in the copyToClipboard() routine
+    if( !m_lockedSelected && !isCut )
     {
         // Second RequestSelection removes locked items but keeps a copy of their pointers
         selectionCopy = m_selectionTool->RequestSelection(
@@ -942,7 +952,10 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
         }
     }
 
-    m_commit->Push( _( "Delete" ) );
+    if( isCut )
+        m_commit->Push( _( "Cut" ) );
+    else
+        m_commit->Push( _( "Delete" ) );
 
     if( !m_lockedSelected && lockedItems.size() > 0 )
     {
@@ -1533,7 +1546,14 @@ int EDIT_TOOL::copyToClipboardWithAnchor( const TOOL_EVENT& aEvent )
 int EDIT_TOOL::cutToClipboard( const TOOL_EVENT& aEvent )
 {
     if( !copyToClipboard( aEvent ) )
-        Remove( aEvent );
+    {
+        // N.B. Setting the CUT flag prevents lock filtering as we only want to delete the items that
+        // were copied to the clipboard, no more, no fewer.  Filtering for locked item, if any will be done
+        // in the copyToClipboard() routine
+        TOOL_EVENT evt = aEvent;
+        evt.SetParameter( PCB_ACTIONS::REMOVE_FLAGS::CUT );
+        Remove( evt );
+    }
 
     return 0;
 }