[gparted] Protect partition members within Operation classes (#759726)



commit 6fd37c074529aaada18ec04baf098133b695b859
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Sat Sep 19 15:57:39 2015 +0100

    Protect partition members within Operation classes (#759726)
    
    The Operation classes contain partition objects which are copied by
    value.  Need to replace these with pointers to Partition objects instead
    and manage their lifetimes so that they can be used polymorphically.
    
    First step is to protect the partition members partition_new,
    partition_original, and for OperationCopy class only, partition_copied
    within the Operation classes and provide accessor methods.
    
    get_partition_new() and get_partition_original() accessors are
    implemented in the Operation base class so all derived classes get an
    implementation.  get_partition_new() is also virtual so that
    OperationCheck and OperationDelete can override the implementation and
    assert that they don't use partition_new.  get_partition_copied() is
    provided for the OperationCopy class only so can only be accessed via an
    OperationCopy type variable.
    
    Bug 759726 - Implement Partition object polymorphism

 include/Operation.h             |   10 ++++-
 include/OperationCheck.h        |    3 ++
 include/OperationCopy.h         |    9 +++--
 include/OperationDelete.h       |    3 ++
 src/GParted_Core.cc             |   69 ++++++++++++++++++++++-----------------
 src/OperationChangeUUID.cc      |    8 ++--
 src/OperationCheck.cc           |   20 ++++++++++-
 src/OperationCreate.cc          |   18 ++++++----
 src/OperationDelete.cc          |   16 +++++++++
 src/OperationFormat.cc          |    6 ++--
 src/OperationLabelFileSystem.cc |    6 ++--
 src/OperationNamePartition.cc   |    6 ++--
 src/OperationResizeMove.cc      |    6 ++--
 src/Win_GParted.cc              |   21 ++++++------
 14 files changed, 129 insertions(+), 72 deletions(-)
---
diff --git a/include/Operation.h b/include/Operation.h
index b2121b3..c91be7f 100644
--- a/include/Operation.h
+++ b/include/Operation.h
@@ -45,6 +45,11 @@ public:
        Operation() ;
        virtual ~Operation() {}
 
+       Partition & get_partition_original()                 { return partition_original; };
+       const Partition & get_partition_original() const     { return partition_original; };
+       virtual Partition & get_partition_new()              { return partition_new; };
+       const virtual Partition & get_partition_new() const  { return partition_new; };
+
        virtual void apply_to_visual( PartitionVector & partitions ) = 0;
        virtual void create_description() = 0 ;
        virtual bool merge_operations( const Operation & candidate ) = 0;
@@ -52,8 +57,6 @@ public:
        //public variables
        Device device ;
        OperationType type ;
-       Partition partition_original ; 
-       Partition partition_new ;
 
        Glib::RefPtr<Gdk::Pixbuf> icon ;
        Glib::ustring description ;
@@ -68,6 +71,9 @@ protected:
                                 Sector start, Sector end, Byte_Value sector_size, bool inside_extended );
        void substitute_new( PartitionVector & partitions );
        void insert_new( PartitionVector & partitions );
+
+       Partition partition_original;
+       Partition partition_new;
 };
 
 } //GParted
diff --git a/include/OperationCheck.h b/include/OperationCheck.h
index 3707d2e..c1ca16a 100644
--- a/include/OperationCheck.h
+++ b/include/OperationCheck.h
@@ -32,6 +32,9 @@ public:
        void apply_to_visual( PartitionVector & partitions );
 
 private:
+       Partition & get_partition_new();
+       const Partition & get_partition_new() const;
+
        void create_description() ;
        bool merge_operations( const Operation & candidate );
 } ;
diff --git a/include/OperationCopy.h b/include/OperationCopy.h
index fb6b62e..734a2fc 100644
--- a/include/OperationCopy.h
+++ b/include/OperationCopy.h
@@ -32,14 +32,17 @@ public:
                       const Partition & partition_new,
                       const Partition & partition_copied ) ;
 
-       void apply_to_visual( PartitionVector & partitions );
+       Partition & get_partition_copied()              { return partition_copied; };
+       const Partition & get_partition_copied() const  { return partition_copied; };
 
-       Partition partition_copied ;
+       void apply_to_visual( PartitionVector & partitions );
 
 private:
        void create_description() ;
        bool merge_operations( const Operation & candidate );
-} ;
+
+       Partition partition_copied;
+};
 
 } //GParted
 
diff --git a/include/OperationDelete.h b/include/OperationDelete.h
index 296abc2..1c9be54 100644
--- a/include/OperationDelete.h
+++ b/include/OperationDelete.h
@@ -32,6 +32,9 @@ public:
        void apply_to_visual( PartitionVector & partitions );
 
 private:
+       Partition & get_partition_new();
+       const Partition & get_partition_new() const;
+
        void create_description() ;
        bool merge_operations( const Operation & candidate );
        void remove_original_and_adjacent_unallocated( PartitionVector & partitions, int index_orig );
diff --git a/src/GParted_Core.cc b/src/GParted_Core.cc
index b5dc7b5..c3b43e6 100644
--- a/src/GParted_Core.cc
+++ b/src/GParted_Core.cc
@@ -729,83 +729,92 @@ bool GParted_Core::apply_operation_to_disk( Operation * operation )
                // modified in the operation results to inform the user.
 
                case OPERATION_DELETE:
-                       success =    calibrate_partition( operation->partition_original, 
operation->operation_detail )
-                                 && remove_filesystem( operation->partition_original, 
operation->operation_detail )
-                                 && Delete( operation->partition_original, operation->operation_detail );
+                       success =    calibrate_partition( operation->get_partition_original(),
+                                                         operation->operation_detail )
+                                 && remove_filesystem( operation->get_partition_original(),
+                                                       operation->operation_detail )
+                                 && Delete( operation->get_partition_original(), operation->operation_detail 
);
                        break;
 
                case OPERATION_CHECK:
-                       success =    calibrate_partition( operation->partition_original, 
operation->operation_detail )
-                                 && check_repair_filesystem( operation->partition_original,
+                       success =    calibrate_partition( operation->get_partition_original(),
+                                                         operation->operation_detail )
+                                 && check_repair_filesystem( operation->get_partition_original(),
                                                              operation->operation_detail )
-                                 && maximize_filesystem( operation->partition_original, 
operation->operation_detail );
+                                 && maximize_filesystem( operation->get_partition_original(),
+                                                         operation->operation_detail );
                        break;
 
                case OPERATION_CREATE:
                        // The partition doesn't exist yet so there's nothing to calibrate.
-                       success = create( operation->partition_new, operation->operation_detail );
+                       success = create( operation->get_partition_new(), operation->operation_detail );
                        break;
 
                case OPERATION_RESIZE_MOVE:
-                       success = calibrate_partition( operation->partition_original, 
operation->operation_detail );
+                       success = calibrate_partition( operation->get_partition_original(),
+                                                      operation->operation_detail );
                        if ( ! success )
                                break;
 
                        // Reset the new partition object's real path in case the name is
                        // "copy of ..." from the previous operation.
-                       operation->partition_new.add_path( operation->partition_original.get_path(), true );
+                       operation->get_partition_new().add_path( 
operation->get_partition_original().get_path(), true );
 
-                       success = resize_move( operation->partition_original,
-                                              operation->partition_new,
+                       success = resize_move( operation->get_partition_original(),
+                                              operation->get_partition_new(),
                                               operation->operation_detail );
                        break;
 
                case OPERATION_FORMAT:
-                       success = calibrate_partition( operation->partition_new, operation->operation_detail 
);
+                       success = calibrate_partition( operation->get_partition_new(), 
operation->operation_detail );
                        if ( ! success )
                                break;
 
                        // Reset the original partition object's real path in case the
                        // name is "copy of ..." from the previous operation.
-                       operation->partition_original.add_path( operation->partition_new.get_path(), true );
+                       operation->get_partition_original().add_path( 
operation->get_partition_new().get_path(), true );
 
-                       success =    remove_filesystem( operation->partition_original, 
operation->operation_detail )
-                                 && format( operation->partition_new, operation->operation_detail );
+                       success =    remove_filesystem( operation->get_partition_original(),
+                                                       operation->operation_detail )
+                                 && format( operation->get_partition_new(), operation->operation_detail );
                        break;
 
                case OPERATION_COPY:
+               {
                        //FIXME: in case of a new partition we should make sure the new partition is >= the 
source partition... 
                        //i think it's best to do this in the dialog_paste
 
-                       success =    calibrate_partition( static_cast<OperationCopy*>( operation 
)->partition_copied,
-                                                         operation->operation_detail )
+                       OperationCopy * copy_op = static_cast<OperationCopy*>( operation );
+                       success =    calibrate_partition( copy_op->get_partition_copied(),
+                                                         copy_op->operation_detail )
                                     // Only calibrate the destination when pasting into an existing
                                     // partition, rather than when creating a new partition.
-                                  && ( operation->partition_original.type == TYPE_UNALLOCATED                
       ||
-                                      calibrate_partition( operation->partition_new, 
operation->operation_detail )    );
+                                  && ( copy_op->get_partition_original().type == TYPE_UNALLOCATED            
         ||
+                                      calibrate_partition( copy_op->get_partition_new(), 
copy_op->operation_detail )    );
                        if ( ! success )
                                break;
 
-                       success =    remove_filesystem( operation->partition_original, 
operation->operation_detail )
-                                 && copy( static_cast<OperationCopy*>( operation )->partition_copied,
-                                          operation->partition_new,
-                                          static_cast<OperationCopy*>( operation 
)->partition_copied.get_byte_length(),
-                                          operation->operation_detail );
+                       success =    remove_filesystem( copy_op->get_partition_original(), 
copy_op->operation_detail )
+                                 && copy( copy_op->get_partition_copied(),
+                                          copy_op->get_partition_new(),
+                                          copy_op->get_partition_copied().get_byte_length(),
+                                          copy_op->operation_detail );
                        break;
+               }
 
                case OPERATION_LABEL_FILESYSTEM:
-                       success =    calibrate_partition( operation->partition_new, 
operation->operation_detail )
-                                 && label_filesystem( operation->partition_new, operation->operation_detail 
);
+                       success =    calibrate_partition( operation->get_partition_new(), 
operation->operation_detail )
+                                 && label_filesystem( operation->get_partition_new(), 
operation->operation_detail );
                        break;
 
                case OPERATION_NAME_PARTITION:
-                       success =    calibrate_partition( operation->partition_new, 
operation->operation_detail )
-                                 && name_partition( operation->partition_new, operation->operation_detail );
+                       success =    calibrate_partition( operation->get_partition_new(), 
operation->operation_detail )
+                                 && name_partition( operation->get_partition_new(), 
operation->operation_detail );
                        break;
 
                case OPERATION_CHANGE_UUID:
-                       success =    calibrate_partition( operation->partition_new, 
operation->operation_detail )
-                                 && change_uuid( operation ->partition_new, operation ->operation_detail ) ;
+                       success =    calibrate_partition( operation->get_partition_new(), 
operation->operation_detail )
+                                 && change_uuid( operation->get_partition_new(), operation->operation_detail 
);
                        break;
        }
 
diff --git a/src/OperationChangeUUID.cc b/src/OperationChangeUUID.cc
index b610764..15dba7b 100644
--- a/src/OperationChangeUUID.cc
+++ b/src/OperationChangeUUID.cc
@@ -57,13 +57,13 @@ void OperationChangeUUID::create_description()
 
 bool OperationChangeUUID::merge_operations( const Operation & candidate )
 {
-       if ( candidate.type == OPERATION_CHANGE_UUID        &&
-            partition_new  == candidate.partition_original    )
+       if ( candidate.type == OPERATION_CHANGE_UUID              &&
+            partition_new  == candidate.get_partition_original()    )
        {
                // Changing half the UUID must not override changing all of it
-               if ( candidate.partition_new.uuid != UUID_RANDOM_NTFS_HALF )
+               if ( candidate.get_partition_new().uuid != UUID_RANDOM_NTFS_HALF )
                {
-                       partition_new.uuid = candidate.partition_new.uuid;
+                       partition_new.uuid = candidate.get_partition_new().uuid;
                        create_description();
                }
                // Else no change required to this operation to merge candidate
diff --git a/src/OperationCheck.cc b/src/OperationCheck.cc
index 058644d..592caa1 100644
--- a/src/OperationCheck.cc
+++ b/src/OperationCheck.cc
@@ -29,6 +29,22 @@ OperationCheck::OperationCheck( const Device & device, const Partition & partiti
        partition_original = partition ;
 }
 
+Partition & OperationCheck::get_partition_new()
+{
+       g_assert( false );  // Bug: OperationCheck class doesn't use partition_new
+
+       // Not reached.  Return value to keep compiler quiet.
+       return partition_new;
+}
+
+const Partition & OperationCheck::get_partition_new() const
+{
+       g_assert( false );  // Bug: OperationCheck class doesn't use partition_new
+
+       // Not reached.  Return value to keep compiler quiet.
+       return partition_new;
+}
+
 void OperationCheck::apply_to_visual( PartitionVector & partitions )
 {
 }
@@ -43,8 +59,8 @@ void OperationCheck::create_description()
 
 bool OperationCheck::merge_operations( const Operation & candidate )
 {
-       if ( candidate.type     == OPERATION_CHECK              &&
-            partition_original == candidate.partition_original    )
+       if ( candidate.type     == OPERATION_CHECK                    &&
+            partition_original == candidate.get_partition_original()    )
                // No steps required to merge this operation
                return true;
 
diff --git a/src/OperationCreate.cc b/src/OperationCreate.cc
index e7deae0..6aedbe2 100644
--- a/src/OperationCreate.cc
+++ b/src/OperationCreate.cc
@@ -16,6 +16,8 @@
  */
 
 #include "../include/OperationCreate.h"
+#include "../include/OperationFormat.h"
+#include "../include/OperationResizeMove.h"
 #include "../include/Partition.h"
 #include "../include/PartitionVector.h"
 
@@ -66,23 +68,23 @@ void OperationCreate::create_description()
 
 bool OperationCreate::merge_operations( const Operation & candidate )
 {
-       if ( candidate.type                      == OPERATION_FORMAT             &&
-            candidate.partition_original.status == STAT_NEW                     &&
-            partition_new                       == candidate.partition_original    )
+       if ( candidate.type                            == OPERATION_FORMAT                   &&
+            candidate.get_partition_original().status == STAT_NEW                           &&
+            partition_new                             == candidate.get_partition_original()    )
        {
                // Merge a format operation on a not yet created partition with the
                // earlier operation which will create it.
-               partition_new = candidate.partition_new;
+               partition_new = candidate.get_partition_new();
                create_description();
                return true;
        }
-       else if ( candidate.type                      == OPERATION_RESIZE_MOVE        &&
-                 candidate.partition_original.status == STAT_NEW                     &&
-                 partition_new                       == candidate.partition_original    )
+       else if ( candidate.type                            == OPERATION_RESIZE_MOVE              &&
+                 candidate.get_partition_original().status == STAT_NEW                           &&
+                 partition_new                             == candidate.get_partition_original()    )
        {
                // Merge a resize/move operation on a not yet created partition with the
                // earlier operation which will create it.
-               partition_new = candidate.partition_new;
+               partition_new = candidate.get_partition_new();
                create_description();
                return true;
        }
diff --git a/src/OperationDelete.cc b/src/OperationDelete.cc
index 77b9e12..545c3c5 100644
--- a/src/OperationDelete.cc
+++ b/src/OperationDelete.cc
@@ -30,6 +30,22 @@ OperationDelete::OperationDelete( const Device & device, const Partition & parti
        this ->partition_original = partition_orig ;
 }
 
+Partition & OperationDelete::get_partition_new()
+{
+       g_assert( false );  // Bug: OperationDelete class doesn't use partition_new
+
+       // Not reached.  Return value to keep compiler quiet.
+       return partition_new;
+}
+
+const Partition & OperationDelete::get_partition_new() const
+{
+       g_assert( false );  // Bug: OperationDelete class doesn't use partition_new
+
+       // Not reached.  Return value to keep compiler quiet.
+       return partition_new;
+}
+
 void OperationDelete::apply_to_visual( PartitionVector & partitions )
 {
        int index_extended;
diff --git a/src/OperationFormat.cc b/src/OperationFormat.cc
index c774f28..86fd000 100644
--- a/src/OperationFormat.cc
+++ b/src/OperationFormat.cc
@@ -65,10 +65,10 @@ void OperationFormat::create_description()
 
 bool OperationFormat::merge_operations( const Operation & candidate )
 {
-       if ( candidate.type == OPERATION_FORMAT             &&
-            partition_new  == candidate.partition_original    )
+       if ( candidate.type == OPERATION_FORMAT                   &&
+            partition_new  == candidate.get_partition_original()    )
        {
-               partition_new = candidate.partition_new;
+               partition_new = candidate.get_partition_new();
                create_description();
                return true;
        }
diff --git a/src/OperationLabelFileSystem.cc b/src/OperationLabelFileSystem.cc
index a91ae4e..f668c7e 100644
--- a/src/OperationLabelFileSystem.cc
+++ b/src/OperationLabelFileSystem.cc
@@ -53,10 +53,10 @@ void OperationLabelFileSystem::create_description()
 
 bool OperationLabelFileSystem::merge_operations( const Operation & candidate )
 {
-       if ( candidate.type == OPERATION_LABEL_FILESYSTEM   &&
-            partition_new  == candidate.partition_original    )
+       if ( candidate.type == OPERATION_LABEL_FILESYSTEM         &&
+            partition_new  == candidate.get_partition_original()    )
        {
-               partition_new.set_filesystem_label( candidate.partition_new.get_filesystem_label() );
+               partition_new.set_filesystem_label( candidate.get_partition_new().get_filesystem_label() );
                create_description();
                return true;
        }
diff --git a/src/OperationNamePartition.cc b/src/OperationNamePartition.cc
index 9414e4c..c2dcfb4 100644
--- a/src/OperationNamePartition.cc
+++ b/src/OperationNamePartition.cc
@@ -56,10 +56,10 @@ void OperationNamePartition::create_description()
 
 bool OperationNamePartition::merge_operations( const Operation & candidate )
 {
-       if ( candidate.type == OPERATION_NAME_PARTITION     &&
-            partition_new  == candidate.partition_original    )
+       if ( candidate.type == OPERATION_NAME_PARTITION           &&
+            partition_new  == candidate.get_partition_original()    )
        {
-               partition_new.name = candidate.partition_new.name;
+               partition_new.name = candidate.get_partition_new().name;
                create_description();
                return true;
        }
diff --git a/src/OperationResizeMove.cc b/src/OperationResizeMove.cc
index 201f46e..23f4050 100644
--- a/src/OperationResizeMove.cc
+++ b/src/OperationResizeMove.cc
@@ -217,10 +217,10 @@ void OperationResizeMove::remove_adjacent_unallocated( PartitionVector & partiti
 
 bool OperationResizeMove::merge_operations( const Operation & candidate )
 {
-       if ( candidate.type == OPERATION_RESIZE_MOVE        &&
-            partition_new  == candidate.partition_original    )
+       if ( candidate.type == OPERATION_RESIZE_MOVE              &&
+            partition_new  == candidate.get_partition_original()    )
        {
-               partition_new = candidate.partition_new;
+               partition_new = candidate.get_partition_new();
                create_description();
                return true;
        }
diff --git a/src/Win_GParted.cc b/src/Win_GParted.cc
index 97008bd..b37326b 100644
--- a/src/Win_GParted.cc
+++ b/src/Win_GParted.cc
@@ -717,7 +717,7 @@ void Win_GParted::Add_Operation( Operation * operation )
                     operation ->type == OPERATION_CHANGE_UUID ||
                     operation ->type == OPERATION_LABEL_FILESYSTEM ||
                     operation ->type == OPERATION_NAME_PARTITION ||
-                    gparted_core .snap_to_alignment( operation ->device, operation ->partition_new, error )
+                    gparted_core.snap_to_alignment( operation->device, operation->get_partition_new(), error 
)
                   )
                {
                        operation ->create_description() ;
@@ -1754,9 +1754,9 @@ void Win_GParted::activate_resize()
 
                // Display warning if moving a non-extended partition which already exists
                // on the disk.
-               if ( operation->partition_original.status       != STAT_NEW                              &&
-                    operation->partition_original.sector_start != operation->partition_new.sector_start &&
-                    operation->partition_original.type         != TYPE_EXTENDED                            )
+               if ( operation->get_partition_original().status       != STAT_NEW                             
       &&
+                    operation->get_partition_original().sector_start != 
operation->get_partition_new().sector_start &&
+                    operation->get_partition_original().type         != TYPE_EXTENDED                        
          )
                {
                        // Warn that move operation might break boot process
                        Gtk::MessageDialog dialog( *this,
@@ -1768,8 +1768,7 @@ void Win_GParted::activate_resize()
                        Glib::ustring tmp_msg =
                                        /*TO TRANSLATORS: looks like   You queued an operation to move the 
start sector of partition /dev/sda3. */
                                        String::ucompose( _( "You have queued an operation to move the start 
sector of partition %1." )
-                                                       , operation ->partition_original .get_path()
-                                                       ) ;
+                                                       , operation->get_partition_original().get_path() );
                        tmp_msg += _("  Failure to boot is most likely to occur if you move the GNU/Linux 
partition containing /boot, or if you move the Windows system partition C:.");
                        tmp_msg += "\n";
                        tmp_msg += _("You can learn how to repair the boot configuration in the GParted 
FAQ.");
@@ -2029,15 +2028,15 @@ void Win_GParted::activate_delete()
        {
                //remove all operations done on this new partition (this includes creation)     
                for ( int t = 0 ; t < static_cast<int>( operations .size() ) ; t++ ) 
-                       if ( operations[t]->partition_new.get_path() == selected_partition_ptr->get_path() )
+                       if ( operations[t]->get_partition_new().get_path() == 
selected_partition_ptr->get_path() )
                                remove_operation( t-- ) ;
                                
                //determine lowest possible new_count
                new_count = 0 ; 
                for ( unsigned int t = 0 ; t < operations .size() ; t++ )
-                       if ( operations[ t ] ->partition_new .status == GParted::STAT_NEW &&
-                            operations[ t ] ->partition_new .partition_number > new_count )
-                               new_count = operations[ t ] ->partition_new .partition_number ;
+                       if ( operations[t]->get_partition_new().status           == STAT_NEW  &&
+                            operations[t]->get_partition_new().partition_number >  new_count    )
+                               new_count = operations[t]->get_partition_new().partition_number;
                        
                new_count += 1 ;
 
@@ -2774,7 +2773,7 @@ int Win_GParted::partition_in_operation_queue_count( const Partition & partition
 
        for ( unsigned int t = 0 ; t < operations .size() ; t++ )
        {
-               if ( partition .get_path() == operations[ t ] ->partition_original .get_path() )
+               if ( partition.get_path() == operations[t]->get_partition_original().get_path() )
                        operation_count++ ;
        }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]