[gparted] Only allow Undo and Apply after merging operations (#699452)

[Curtis Gedak <gedakc src gnome org>]

commit 4cc426c6cfb579548d432a90f12189494160a5f5
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Fri May 17 17:01:42 2013 +0100

    Only allow Undo and Apply after merging operations (#699452)
    
    It was possible to make GParted crash by adding a label, check or new
    UUID operation and then applying the operation before the view of
    pending operations had finished fully opening.  The operation would be
    successfully applied but GParted would crash afterwards.
    
    The fault was that Add_Operation() still enabled the Undo and Apply
    buttons and processed the GTK event loop before merging the list of
    pending operations.  Faulty code flow went like this:
    
        activate_*()
            Add_Operation()
                Add operation to the operations[] vector
                Enable Undo and Apply buttons
                Refresh_Visual()
                    Process GTK event loop
                        Process Apply button callback applying operations,
                        refreshing display and clearing operations[] vector
            Merge operations in the operations[] vector
            << Core dump here >>
                Merge_Operations()
                    Refresh_Visual()
    
    This faulty code flow came about when merging of operations was added
    and it didn't appreciate that the operations[] vector could have been
    processed and cleared by Add_Operations() before the merge step.
    Relevant commit:
    
        b10349ae37df06b7bf7de91ea93a3913535ade3a
        Merge overlapping operations (#438573)
    
    Fragment of code in the label operation case:
    
      2454  void Win_GParted::activate_label_partition()
      2455  {
      ...
      2472          Add_Operation( operation ) ;
      2473
      2474          // Verify if the two operations can be merged
      2475          for ( unsigned int t = 0 ; t < operations .size() - 1 ; t++ )
      2476          {
      2477              if ( operations[ t ] ->type == OPERATION_LABEL_PARTITION )
      2478              {
      2479                  if ( Merge_Operations( t, operations .size() - 1 ) )
      2480                      break;
      2481              }
      2482          }
    
    Commentary in the crashing label operation case:
    
      2472  The pending operation was already applied when Add_Operation()
            returned resulting in the operations[] vector being cleared
            setting its size to 0.
      2475  The return type of operations.size() is an unsigned integral, so
            the upper limit of the for loop is t < 0UL - 1.  Assuming a
            32-bit machine that's t < 4294967295.
      2477  operations[] vector is access from out of bounds offset 0
            upwards until unallocated memory is accessed resulting in a core
            dump.
    
    Fix this by not enabling the Undo and Apply buttons and processing the
    GTK event loop until after merging of operations has been performed.
    Fixed code flow goes like this:
    
        activate_*()
            Add_Operation()
                Add operation to the operations[] vector
            Merge operations in the operations[] vector
                Merge_Operations()
            show_operationslist()
                Enable Undo and Apply buttons
                Refresh_Visual()
                    Process GTK event loop
                        Process Apply button callback applying operations,
                        refreshing display and clearing operations[] vector
    
    Not allowing the operations list to be process until after the merge
    step is the be correct ordering.  This also prevents the new operation
    from flashing up in the operations list and then immediately
    disappearing if merged.  In the case of adding the first operation,
    delaying enabling the Undo and Apply buttons is enough as the buttons
    were previously disabled preventing the operation being applied before
    the merge.  In the case of adding further operations, processing of the
    GTK event loop must also be delayed until after the merge to prevent the
    operations being applied before the merge.  Although that window of
    opportunity would only be microseconds.
    
    Bug #699452 - Crash when applying operations before pending operations
                  fully displayed

 include/Win_GParted.h |    1 +
 src/Win_GParted.cc    |   64 +++++++++++++++++++++++++++++--------------------
 2 files changed, 39 insertions(+), 26 deletions(-)
---
diff --git a/include/Win_GParted.h b/include/Win_GParted.h
index 4f1958a..05c1595 100644
--- a/include/Win_GParted.h
+++ b/include/Win_GParted.h
@@ -65,6 +65,7 @@ private:
        void Refresh_Visual();
        bool Quit_Check_Operations();
        void set_valid_operations() ;
+       void show_operationslist() ;
        
        //convenience functions
        void toggle_item( bool state, int menu_item, int toolbar_item = -1 )
diff --git a/src/Win_GParted.cc b/src/Win_GParted.cc
index 4201e21..349bac6 100644
--- a/src/Win_GParted.cc
+++ b/src/Win_GParted.cc
@@ -714,22 +714,6 @@ void Win_GParted::Add_Operation( Operation * operation, int index )
                                operations .insert( operations .begin() + index, operation ) ;
                        else
                                operations .push_back( operation );
-
-                       allow_undo_clear_apply( true ) ;
-                       Refresh_Visual();
-                       
-                       if ( operations .size() == 1 ) //first operation, open operationslist
-                               open_operationslist() ;
-
-                       //FIXME:  A slight flicker may be introduced by this extra display refresh.
-                       //An extra display refresh seems to prevent the disk area visual disk from
-                       //  disappearing when enough operations are added to require a scrollbar
-                       //  (about 4 operations with default window size).
-                       //  Note that commenting out the code to
-                       //  "//make scrollwindow focus on the last operation in the list"
-                       //  in HBoxOperations::load_operations() prevents this problem from occurring as well.
-                       //  See also Win_GParted::activate_undo().
-                       drawingarea_visualdisk .queue_draw() ;
                }
                else
                {
@@ -761,8 +745,6 @@ bool Win_GParted::Merge_Operations( unsigned int first, unsigned int second )
                operations[ first ]->create_description() ;
                remove_operation( second );
 
-               Refresh_Visual();
-
                return true;
        }
        // Two label change operations on the same partition
@@ -775,8 +757,6 @@ bool Win_GParted::Merge_Operations( unsigned int first, unsigned int second )
                operations[ first ]->create_description() ;
                remove_operation( second );
 
-               Refresh_Visual();
-
                return true;
        }
        // Two change-uuid change operations on the same partition
@@ -792,8 +772,6 @@ bool Win_GParted::Merge_Operations( unsigned int first, unsigned int second )
                operations[ first ]->create_description() ;
                remove_operation( second );
 
-               Refresh_Visual();
-
                return true;
        }
        // Two check operations of the same partition
@@ -804,8 +782,6 @@ bool Win_GParted::Merge_Operations( unsigned int first, unsigned int second )
        {
                remove_operation( second );
 
-               Refresh_Visual();
-
                return true;
        }
        // Two format operations of the same partition
@@ -818,8 +794,6 @@ bool Win_GParted::Merge_Operations( unsigned int first, unsigned int second )
                operations[ first ]->create_description() ;
                remove_operation( second );
 
-               Refresh_Visual();
-
                return true;
        }
 
@@ -1157,6 +1131,28 @@ void Win_GParted::set_valid_operations()
        }
 }
 
+void Win_GParted::show_operationslist()
+{
+       //Enable (or disable) Undo and Apply buttons
+       allow_undo_clear_apply( operations .size() ) ;
+
+       //Updates view of operations list and sensitivity of Undo and Apply buttons
+       Refresh_Visual();
+
+       if ( operations .size() == 1 ) //first operation, open operationslist
+               open_operationslist() ;
+
+       //FIXME:  A slight flicker may be introduced by this extra display refresh.
+       //An extra display refresh seems to prevent the disk area visual disk from
+       //  disappearing when enough operations are added to require a scrollbar
+       //  (about 4 operations with default window size).
+       //  Note that commenting out the code to
+       //  "//make scrollwindow focus on the last operation in the list"
+       //  in HBoxOperations::load_operations() prevents this problem from occurring as well.
+       //  See also Win_GParted::activate_undo().
+       drawingarea_visualdisk .queue_draw() ;
+}
+
 void Win_GParted::open_operationslist() 
 {
        if ( ! OPERATIONSLIST_OPEN )
@@ -1663,6 +1659,8 @@ void Win_GParted::activate_resize()
                        }
                }
        }
+
+       show_operationslist() ;
 }
 
 void Win_GParted::activate_copy()
@@ -1768,6 +1766,8 @@ void Win_GParted::activate_paste()
                        dialog .run() ;
                }
        }
+
+       show_operationslist() ;
 }
 
 void Win_GParted::activate_new()
@@ -1801,6 +1801,8 @@ void Win_GParted::activate_new()
                        operation ->icon = render_icon( Gtk::Stock::NEW, Gtk::ICON_SIZE_MENU );
 
                        Add_Operation( operation );
+
+                       show_operationslist() ;
                }
        }
 }
@@ -1908,6 +1910,8 @@ void Win_GParted::activate_delete()
 
                Add_Operation( operation ) ;
        }
+
+       show_operationslist() ;
 }
 
 void Win_GParted::activate_info()
@@ -2023,6 +2027,8 @@ void Win_GParted::activate_format( GParted::FILESYSTEM new_fs )
                        Merge_Operations( operations .size() - 2, operations .size() - 1 );
                }
        }
+
+       show_operationslist() ;
 }
 
 void Win_GParted::unmount_partition( bool * succes, Glib::ustring * error ) 
@@ -2449,6 +2455,8 @@ void Win_GParted::activate_check()
                                break;
                }
        }
+
+       show_operationslist() ;
 }
 
 void Win_GParted::activate_label_partition() 
@@ -2480,6 +2488,8 @@ void Win_GParted::activate_label_partition()
                                        break;
                        }
                }
+
+               show_operationslist() ;
        }
 }
        
@@ -2530,6 +2540,8 @@ void Win_GParted::activate_change_uuid()
                                break;
                }
        }
+
+       show_operationslist() ;
 }
 
 void Win_GParted::activate_undo()