[gparted] Refactor operation cases in apply_operation_to_disk() (#746559)



commit cca8a55f51da012b412064f2db9f4e60056afc5e
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Sun Mar 22 20:41:53 2015 +0000

    Refactor operation cases in apply_operation_to_disk() (#746559)
    
    Background:
    
    GParted_Core::calibrate_partition() reloads the partition path name and
    boundary to ensure they are correct before the operation is performed.
    (See comments in calibrate_partition() for the reasons why this is
    necessary).  This also displays details of the partition being modified
    in the operation details to inform the user.
    
    The operation object contains these relevant member objects:
    
      * partition_original
        Partition before the operation is applied.
    
      * partition_new
        Partition as it is intended to be after the operation has been
        applied.
    
      * partition_copied (for the copy operation only)
        Source partition being copied.
    
    Issues:
    
    GParted_Core::apply_operation_to_disk() was always calibrating partition
    object partition_original, but for about half the operations
    partition_original was not used and partition_new is used, so should be
    calibrated instead.
    
    Copy into an existing partition calibrated three partitions, the source,
    destination before and destination after the operation was applied.
    This doesn't really make sense in the operation details to the user.
    They would expect to only see the source and destination partitions and
    don't care about the distinction between the before and after
    representation of the destination.
    
    Minor issues:
    
    The previous fix had to copy the correct partition path from the
    calibrated partition_original object to the used partition_new object
    for the format, label file system, name partition and change uuid
    operations.
    
    Calibrate was called for the create operation too, even though the
    partition didn't yet exist.  It was a no-operation.
    
    Fix:
    
    Stop always calibrating the partition_original object and instead
    calibrate the correct partition object in each operation case.  For the
    copy into existing partition operation only calibrate the right two
    partition objects as the user would expect.
    
    Bug 746559 - Various operations fail when following paste into existing
                 partition

 src/GParted_Core.cc |  145 +++++++++++++++++++++++++++++++--------------------
 1 files changed, 89 insertions(+), 56 deletions(-)
---
diff --git a/src/GParted_Core.cc b/src/GParted_Core.cc
index 8a8faf9..cf45fab 100644
--- a/src/GParted_Core.cc
+++ b/src/GParted_Core.cc
@@ -739,66 +739,99 @@ bool GParted_Core::snap_to_alignment( const Device & device, Partition & partiti
 
 bool GParted_Core::apply_operation_to_disk( Operation * operation )
 {
-       bool succes = false ;
+       bool success = false;
        libparted_messages .clear() ;
 
-       if ( calibrate_partition( operation ->partition_original, operation ->operation_detail ) )
-               switch ( operation ->type )
-               {            
-                       case OPERATION_DELETE:
-                               succes = remove_filesystem( operation ->partition_original, operation 
->operation_detail ) &&
-                                        Delete( operation ->partition_original, operation ->operation_detail 
) ;
-                               break ;
-                       case OPERATION_CHECK:
-                               succes = check_repair_filesystem( operation ->partition_original, operation 
->operation_detail ) &&
-                                        maximize_filesystem( operation ->partition_original, operation 
->operation_detail ) ;
-                               break ;
-                       case OPERATION_CREATE:
-                               succes = create( operation->partition_new, operation->operation_detail );
-                               break ;
-                       case OPERATION_RESIZE_MOVE:
-                               //in case the to be resized/moved partition was a 'copy of..', we need a real 
path...
-                               operation ->partition_new .add_path( operation ->partition_original 
.get_path(), true ) ;
-                               succes = resize_move( operation ->partition_original,
-                                                     operation ->partition_new,
-                                                     operation ->operation_detail );
-                               break ;
-                       case OPERATION_FORMAT:
-                               // Reset real path in case the name is "copy of ..." from previous operation
-                               operation->partition_new.add_path( operation->partition_original.get_path(), 
true );
-                               succes = remove_filesystem( operation ->partition_original, operation 
->operation_detail ) &&
-                                        format( operation ->partition_new, operation ->operation_detail ) ;
-                               break ;
-                       case OPERATION_COPY:
+       switch ( operation->type )
+       {
+               // Call calibrate_partition() first for each operation to ensure the
+               // correct partition path name and boundary is known before performing the
+               // actual modifications.  (See comments in calibrate_partition() for
+               // reasons why this is necessary).  Calibrate the most relevant partition
+               // object(s), either partition_original, partition_new or partition_copy,
+               // as required.  Calibrate also displays details of the partition being
+               // 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 );
+                       break;
+
+               case OPERATION_CHECK:
+                       success =    calibrate_partition( operation->partition_original, 
operation->operation_detail )
+                                 && check_repair_filesystem( operation->partition_original,
+                                                             operation->operation_detail )
+                                 && maximize_filesystem( operation->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 );
+                       break;
+
+               case OPERATION_RESIZE_MOVE:
+                       success = calibrate_partition( operation->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 );
+
+                       success = resize_move( operation->partition_original,
+                                              operation->partition_new,
+                                              operation->operation_detail );
+                       break;
+
+               case OPERATION_FORMAT:
+                       success = calibrate_partition( operation->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 );
+
+                       success =    remove_filesystem( operation->partition_original, 
operation->operation_detail )
+                                 && format( operation->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
-                               succes = ( operation ->partition_original .type == TYPE_UNALLOCATED || 
-                                          calibrate_partition( operation ->partition_new, operation 
->operation_detail ) ) &&
-                               
-                                        calibrate_partition( static_cast<OperationCopy*>( operation ) 
->partition_copied,
-                                                             operation ->operation_detail ) &&
-                                        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 ) ;
-                               break ;
-                       case OPERATION_LABEL_FILESYSTEM:
-                               // Reset real path in case the name is "copy of ..." from previous operation
-                               operation->partition_new.add_path( operation->partition_original.get_path(), 
true );
-                               succes = label_filesystem( operation->partition_new, 
operation->operation_detail );
-                               break ;
-                       case OPERATION_NAME_PARTITION:
-                               // Reset real path in case the name is "copy of ..." from previous operation
-                               operation->partition_new.add_path( operation->partition_original.get_path(), 
true );
-                               succes = name_partition( operation->partition_new, 
operation->operation_detail );
+
+                       success =    calibrate_partition( static_cast<OperationCopy*>( operation 
)->partition_copied,
+                                                         operation->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 )    );
+                       if ( ! success )
                                break;
-                       case OPERATION_CHANGE_UUID:
-                               // Reset real path in case the name is "copy of ..." from previous operation
-                               operation->partition_new.add_path( operation->partition_original.get_path(), 
true );
-                               succes = change_uuid( operation ->partition_new, operation ->operation_detail 
) ;
-                               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 );
+                       break;
+
+               case OPERATION_LABEL_FILESYSTEM:
+                       success =    calibrate_partition( operation->partition_new, 
operation->operation_detail )
+                                 && label_filesystem( operation->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 );
+                       break;
+
+               case OPERATION_CHANGE_UUID:
+                       success =    calibrate_partition( operation->partition_new, 
operation->operation_detail )
+                                 && change_uuid( operation ->partition_new, operation ->operation_detail ) ;
+                       break;
+       }
 
        if ( libparted_messages .size() > 0 )
        {
@@ -809,7 +842,7 @@ bool GParted_Core::apply_operation_to_disk( Operation * operation )
                                OperationDetail( libparted_messages[ t ], STATUS_NONE, FONT_ITALIC ) ) ;
        }
 
-       return succes ;
+       return success;
 }
 
 bool GParted_Core::set_disklabel( const Device & device, const Glib::ustring & disklabel )


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