Prevent segfault for AssumeDeleted when parent and child are selected…

… (GitHub issue #149)

src/Cleanup.cpp

@@ -80,33 +80,12 @@ void Cleanup::execute( FileInfo *item, OutputWindow * outputWindow )
 {
     if ( worksFor( item ) )
     {
-	DirTree * tree = item->tree();
-
 	executeRecursive( item, outputWindow );
 
-	switch ( _refreshPolicy )
-	{
-	    case NoRefresh:
-		// Do nothing (by definition).
-		break;
-
-	    case RefreshThis:
-	    case RefreshParent:
-		// Done from CleanupCollection::execute() via a Refresher
-		// object that is triggered by the
-		// OutputWindow::lastProcessFinished() signal.
-		//
-		// Nothing left to do here.
-		break;
-
-	    case AssumeDeleted:
-
-		// Assume the cleanup action has deleted the item.
-		// Modify the DirTree accordingly.
-
-		tree->deleteSubtree( item );
-		break;
-	}
+        // Refreshing the tree based on the cleanup's refresh policy is now
+        // handled completely in CleanupCollection::execute() to be safer
+        // against segfaults due to pointers becoming invalid due to their
+        // parents having been deleted during refreshing.
     }
 }
 

src/CleanupCollection.cpp

@@ -14,6 +14,7 @@
 #include "CleanupCollection.h"
 #include "Cleanup.h"
 #include "StdCleanup.h"
+#include "DirTree.h"
 #include "Settings.h"
 #include "SettingsHelpers.h"
 #include "SelectionModel.h"
@@ -290,6 +291,12 @@ void CleanupCollection::execute()
     connect( outputWindow, SIGNAL( lastProcessFinished( int ) ),
 	     this,	   SIGNAL( cleanupFinished    ( int ) ) );
 
+
+    // Intentionally not using the normalized FileInfoSet here: If a user
+    // selects a file and one of its ancestors, he might be interested to
+    // perform an action on each of them individually. We can't know if the
+    // action on the ancestor affects any of its children.
+
     foreach ( FileInfo * item, selection )
     {
 	if ( cleanup->worksFor( item ) )
@@ -303,6 +310,19 @@ void CleanupCollection::execute()
 	}
     }
 
+    if ( cleanup->refreshPolicy() == Cleanup::AssumeDeleted )
+    {
+        // It is important to use the normalized FileInfoSet here to avoid a
+        // segfault because we are iterating over items whose ancestors we just
+        // deleted (thus invalidating pointers to it). Normalizing removes
+        // items from the set that also have any ancestors in the set.
+
+        foreach ( FileInfo * item, selection.normalized() )
+        {
+            item->tree()->deleteSubtree( item );
+        }
+    }
+
     outputWindow->noMoreProcesses();
 }