GNOME.org
 

[gparted] Refactor flags in method FileSystem::execute_command() (#754684)

commit 83ecae49181a572e2bee67b73e290ed7ab2f62e7
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Sat Aug 29 15:15:24 2015 +0100

    Refactor flags in method FileSystem::execute_command() (#754684)
    
    Change the two optional boolean parameters into a single optional flags
    parameter which uses symbolically defined names.  Makes reading the
    execute_command() calls much easier to understand.  (Implemented as bit
    field using the same technique as used for Glib::SpawnFlags [1]).
    
    This changes the calls thus:
    
      execute_command(cmd, od)              -> (cmd, od)
      execute_command(cmd, od, false)       -> (cmd, od, EXEC_NONE)  // [2]
      execute_command(cmd, od, true )       -> (cmd, od, EXEC_CHECK_STATUS)
      execute_command(cmd, od, false, true) -> (cmd, od, EXEC_CANCEL_SAFE)
      execute_command(cmd, od, true , true) ->
                              (cmd, od, EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE)
    
    [1] SpawnFlags bitwise operators in
        /usr/include/glibmm-2.4/glibmm/spawn.h.
    
    [2] False and EXEC_NONE are the default values for the optional third
        parameter before and after this change respectively and both mean
        the same.  This is being used in btrfs::resize() and being kept for
        now despite it being the default.
    
    Bug 754684 - Updates to FileSystem:: and Utils::execute_command()
                 functions

 include/FileSystem.h |   21 ++++++++++++++++++++-
 src/FileSystem.cc    |   11 +++++++----
 src/btrfs.cc         |    7 ++++---
 src/ext2.cc          |   10 +++++-----
 src/fat16.cc         |    5 ++---
 src/jfs.cc           |   14 ++++++--------
 src/nilfs2.cc        |    6 +++---
 src/ntfs.cc          |    9 +++------
 src/reiser4.cc       |    6 ++----
 src/reiserfs.cc      |    6 +++---
 src/xfs.cc           |   29 ++++++++++++++---------------
 11 files changed, 69 insertions(+), 55 deletions(-)
---
diff --git a/include/FileSystem.h b/include/FileSystem.h
index 18191f5..5c72fbf 100644
--- a/include/FileSystem.h
+++ b/include/FileSystem.h
@@ -28,6 +28,25 @@
 namespace GParted
 {
 
+enum ExecFlags
+{
+       EXEC_NONE         = 1 << 0,
+       EXEC_CHECK_STATUS = 1 << 1,  // Time and check exit status of the command in
+                                    // operation details.  Only used when multiple
+                                    // commands are executed in the same file system
+                                    // specific action method.  (GParted_Core displays
+                                    // the time and success of each action method in the
+                                    // parent operation detail so don't bother for single
+                                    // command file system action methods).
+       EXEC_CANCEL_SAFE  = 1 << 2
+};
+
+inline ExecFlags operator|( ExecFlags lhs, ExecFlags rhs )
+       { return static_cast<ExecFlags>( static_cast<unsigned>(lhs) | static_cast<unsigned>(rhs) ); }
+
+inline ExecFlags operator&( ExecFlags lhs, ExecFlags rhs )
+       { return static_cast<ExecFlags>( static_cast<unsigned>(lhs) & static_cast<unsigned>(rhs) ); }
+
 class FileSystem
 {
 public:
@@ -60,7 +79,7 @@ public:
        bool success;
 protected:
        int execute_command( const Glib::ustring & command, OperationDetail & operationdetail,
-                            bool checkstatus = false, bool cancel_safe = false );
+                            ExecFlags flags = EXEC_NONE );
        void execute_command_eof();
        Glib::ustring mk_temp_dir( const Glib::ustring & infix, OperationDetail & operationdetail ) ;
        void rm_temp_dir( const Glib::ustring dir_name, OperationDetail & operationdetail ) ;
diff --git a/src/FileSystem.cc b/src/FileSystem.cc
index 6b7f16c..a63d688 100644
--- a/src/FileSystem.cc
+++ b/src/FileSystem.cc
@@ -80,9 +80,11 @@ static void setup_child()
 }
 
 int FileSystem::execute_command( const Glib::ustring & command, OperationDetail & operationdetail,
-                                bool checkstatus, bool cancel_safe )
+                                 ExecFlags flags )
 {
-       operationdetail .add_child( OperationDetail( command, checkstatus ? STATUS_EXECUTE : STATUS_NONE, 
FONT_BOLD_ITALIC ) ) ;
+       operationdetail.add_child( OperationDetail( command,
+                                                   ( flags & EXEC_CHECK_STATUS ) ? STATUS_EXECUTE : 
STATUS_NONE,
+                                                   FONT_BOLD_ITALIC ) );
        Glib::Pid pid;
        // set up pipes for capture
        int out, err;
@@ -132,10 +134,11 @@ int FileSystem::execute_command( const Glib::ustring & command, OperationDetail
                sigc::bind(
                        sigc::ptr_fun( cancel_command ),
                        pid,
-                       cancel_safe ));
+                       flags & EXEC_CANCEL_SAFE ) );
        Gtk::Main::run();
 
-       if (checkstatus) {
+       if ( flags & EXEC_CHECK_STATUS )
+       {
                if ( !exit_status )
                        operationdetail.get_last_child().set_status( STATUS_SUCCES );
                else
diff --git a/src/btrfs.cc b/src/btrfs.cc
index a1fee53..360ca23 100644
--- a/src/btrfs.cc
+++ b/src/btrfs.cc
@@ -318,7 +318,7 @@ bool btrfs::resize( const Partition & partition_new, OperationDetail & operation
                if ( mount_point .empty() )
                        return false ;
                success &= ! execute_command( "mount -v -t btrfs " + path + " " + mount_point,
-                                             operationdetail, true ) ;
+                                             operationdetail, EXEC_CHECK_STATUS );
        }
        else
                mount_point = partition_new .get_mountpoint() ;
@@ -336,7 +336,7 @@ bool btrfs::resize( const Partition & partition_new, OperationDetail & operation
                        cmd = "btrfs filesystem resize " + devid_str + ":" + size + " " + mount_point ;
                else
                        cmd = "btrfsctl -r " + devid_str + ":" + size + " " + mount_point ;
-               exit_status = execute_command( cmd, operationdetail, false ) ;
+               exit_status = execute_command( cmd, operationdetail, EXEC_NONE );
                bool resize_succeeded = ( exit_status == 0 ) ;
                if ( resize_to_same_size_fails )
                {
@@ -362,7 +362,8 @@ bool btrfs::resize( const Partition & partition_new, OperationDetail & operation
                success &= resize_succeeded ;
 
                if ( ! partition_new .busy )
-                       success &= ! execute_command( "umount -v " + mount_point, operationdetail, true ) ;
+                       success &= ! execute_command( "umount -v " + mount_point, operationdetail,
+                                                     EXEC_CHECK_STATUS );
        }
 
        if ( ! partition_new .busy )
diff --git a/src/ext2.cc b/src/ext2.cc
index 591d04c..e4a3f64 100644
--- a/src/ext2.cc
+++ b/src/ext2.cc
@@ -220,7 +220,7 @@ bool ext2::create( const Partition & new_partition, OperationDetail & operationd
 {
        return ! execute_command( mkfs_cmd + " -F -L \"" + new_partition.get_filesystem_label() + "\" " +
                                  new_partition.get_path(),
-                                 operationdetail, false, true );
+                                 operationdetail, EXEC_CANCEL_SAFE );
 }
 
 bool ext2::resize( const Partition & partition_new, OperationDetail & operationdetail, bool fill_partition )
@@ -237,7 +237,7 @@ bool ext2::resize( const Partition & partition_new, OperationDetail & operationd
 bool ext2::check_repair( const Partition & partition, OperationDetail & operationdetail )
 {
        exit_status = execute_command( fsck_cmd + " -f -y -v -C 0 " + partition.get_path(), operationdetail,
-                                      false, true );
+                                      EXEC_CANCEL_SAFE );
 
        //exitstatus 256 isn't documented, but it's returned when the 'FILE SYSTEM IS MODIFIED'
        //this is quite normal (especially after a copy) so we let the function return true...
@@ -254,10 +254,10 @@ bool ext2::move( const Partition & partition_new,
        offset = Utils::num_to_str( llabs(distance) * partition_new.sector_size );
        if ( distance < 0 )
                return ! execute_command( image_cmd + " -ra -p -o " + offset + " " + partition_new.get_path(),
-                                         operationdetail, true, true );
+                                         operationdetail, EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE );
        else
                return ! execute_command( image_cmd + " -ra -p -O " + offset + " " + partition_new.get_path(),
-                                         operationdetail, true, true );
+                                         operationdetail, EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE );
 
 }
 
@@ -266,7 +266,7 @@ bool ext2::copy( const Partition & src_part,
                  OperationDetail & operationdetail )
 {
        return ! execute_command( image_cmd + " -ra -p " + src_part.get_path() + " " + dest_part.get_path(),
-                                 operationdetail, true, true );
+                                 operationdetail, EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE );
 }
 
 } //GParted
diff --git a/src/fat16.cc b/src/fat16.cc
index c12bfd7..d0dd886 100644
--- a/src/fat16.cc
+++ b/src/fat16.cc
@@ -252,14 +252,13 @@ bool fat16::create( const Partition & new_partition, OperationDetail & operation
                                  pad_label( new_partition.get_filesystem_label() ) + "\" " +
                                  new_partition.get_path(),
                                  operationdetail,
-                                 false,
-                                 true );
+                                 EXEC_CANCEL_SAFE );
 }
 
 bool fat16::check_repair( const Partition & partition, OperationDetail & operationdetail )
 {
        exit_status = execute_command( check_cmd + " -a -w -v " + partition .get_path(), operationdetail,
-                                      false, true );
+                                      EXEC_CANCEL_SAFE );
 
        return ( exit_status == 0 || exit_status == 1 || exit_status == 256 ) ;
 }
diff --git a/src/jfs.cc b/src/jfs.cc
index 83d1254..b400a6c 100644
--- a/src/jfs.cc
+++ b/src/jfs.cc
@@ -159,8 +159,8 @@ bool jfs::write_uuid( const Partition & partition, OperationDetail & operationde
 bool jfs::create( const Partition & new_partition, OperationDetail & operationdetail )
 {
        return ! execute_command( "mkfs.jfs -q -L \"" + new_partition.get_filesystem_label() + "\" " +
-                                 new_partition.get_path(), operationdetail,
-                                 false, true );
+                                 new_partition.get_path(), operationdetail,
+                                 EXEC_CANCEL_SAFE );
 }
 
 bool jfs::resize( const Partition & partition_new, OperationDetail & operationdetail, bool fill_partition )
@@ -174,7 +174,7 @@ bool jfs::resize( const Partition & partition_new, OperationDetail & operationde
                if ( mount_point .empty() )
                        return false ;
                success &= ! execute_command( "mount -v -t jfs " + partition_new .get_path() + " " + 
mount_point,
-                                             operationdetail, true ) ;
+                                             operationdetail, EXEC_CHECK_STATUS );
        }
        else
                mount_point = partition_new .get_mountpoint() ;
@@ -182,10 +182,10 @@ bool jfs::resize( const Partition & partition_new, OperationDetail & operationde
        if ( success )
        {
                success &= ! execute_command( "mount -v -t jfs -o remount,resize " + partition_new 
.get_path() + " " + mount_point,
-                                             operationdetail, true ) ;
+                                             operationdetail, EXEC_CHECK_STATUS );
 
                if ( ! partition_new .busy )
-                       success &= ! execute_command( "umount -v " + mount_point, operationdetail, true ) ;
+                       success &= ! execute_command( "umount -v " + mount_point, operationdetail, 
EXEC_CHECK_STATUS );
        }
 
        if ( ! partition_new .busy )
@@ -197,11 +197,9 @@ bool jfs::resize( const Partition & partition_new, OperationDetail & operationde
 bool jfs::check_repair( const Partition & partition, OperationDetail & operationdetail )
 {
        exit_status = execute_command( "jfs_fsck -f " + partition.get_path(), operationdetail,
-                                      false, true );
+                                      EXEC_CANCEL_SAFE );
 
        return ( exit_status == 0 || exit_status == 1 ) ;
 }
 
 } //GParted
-
-
diff --git a/src/nilfs2.cc b/src/nilfs2.cc
index 1ef04e4..7aa6a03 100644
--- a/src/nilfs2.cc
+++ b/src/nilfs2.cc
@@ -180,7 +180,7 @@ bool nilfs2::resize( const Partition & partition_new, OperationDetail & operatio
                        return false ;
 
                success &= ! execute_command( "mount -v -t nilfs2 " + partition_new .get_path() + " " + 
mount_point,
-                                             operationdetail, true ) ;
+                                             operationdetail, EXEC_CHECK_STATUS );
        }
 
        if ( success )
@@ -192,10 +192,10 @@ bool nilfs2::resize( const Partition & partition_new, OperationDetail & operatio
                                        partition_new .get_sector_length(), partition_new .sector_size, 
UNIT_KIB ) ) ) + "K" ;
                        cmd += " " + size ;
                }
-               success &= ! execute_command( cmd, operationdetail, true ) ;
+               success &= ! execute_command( cmd, operationdetail, EXEC_CHECK_STATUS );
 
                if ( ! partition_new. busy )
-                       success &= ! execute_command( "umount -v " + mount_point, operationdetail, true ) ;
+                       success &= ! execute_command( "umount -v " + mount_point, operationdetail, 
EXEC_CHECK_STATUS );
        }
 
        if ( ! partition_new .busy )
diff --git a/src/ntfs.cc b/src/ntfs.cc
index 96f147f..f757d9b 100644
--- a/src/ntfs.cc
+++ b/src/ntfs.cc
@@ -196,7 +196,7 @@ bool ntfs::create( const Partition & new_partition, OperationDetail & operationd
 {
        return ! execute_command( "mkntfs -Q -v -F -L \"" + new_partition.get_filesystem_label() + "\" " +
                                  new_partition.get_path(),
-                                 operationdetail, false, true );
+                                 operationdetail, EXEC_CANCEL_SAFE );
 }
 
 bool ntfs::resize( const Partition & partition_new, OperationDetail & operationdetail, bool fill_partition )
@@ -243,9 +243,8 @@ bool ntfs::copy( const Partition & src_part,
                 OperationDetail & operationdetail )
 {
        return ! execute_command( "ntfsclone -f --overwrite " + dest_part.get_path() + " " + 
src_part.get_path(),
-                                 operationdetail,
-                                 false,
-                                 true );
+                                 operationdetail,
+                                 EXEC_CANCEL_SAFE );
 }
 
 bool ntfs::check_repair( const Partition & partition, OperationDetail & operationdetail )
@@ -254,5 +253,3 @@ bool ntfs::check_repair( const Partition & partition, OperationDetail & operatio
 }
 
 } //GParted
-
-
diff --git a/src/reiser4.cc b/src/reiser4.cc
index 5874b1a..392aba5 100644
--- a/src/reiser4.cc
+++ b/src/reiser4.cc
@@ -140,15 +140,13 @@ bool reiser4::create( const Partition & new_partition, OperationDetail & operati
 {
        return ! execute_command( "mkfs.reiser4 --force --yes --label \"" + 
new_partition.get_filesystem_label() + "\" " +
                                  new_partition.get_path(), operationdetail,
-                                 false, true );
+                                 EXEC_CANCEL_SAFE );
 }
 
 bool reiser4::check_repair( const Partition & partition, OperationDetail & operationdetail )
 {
        return ! execute_command( "fsck.reiser4 --yes --fix --quiet " + partition.get_path(),
-                                 operationdetail, false, true );
+                                 operationdetail, EXEC_CANCEL_SAFE );
 }
 
 } //GParted
-
-
diff --git a/src/reiserfs.cc b/src/reiserfs.cc
index 7bb4f0c..89d6bab 100644
--- a/src/reiserfs.cc
+++ b/src/reiserfs.cc
@@ -161,7 +161,7 @@ bool reiserfs::create( const Partition & new_partition, OperationDetail & operat
 {
        return ! execute_command( "mkreiserfs -f -f --label \"" + new_partition.get_filesystem_label() + "\" 
" +
                                  new_partition.get_path(),
-                                 operationdetail, false, true );
+                                 operationdetail, EXEC_CANCEL_SAFE );
 }
 
 bool reiserfs::resize( const Partition & partition_new, OperationDetail & operationdetail, bool 
fill_partition )
@@ -182,8 +182,8 @@ bool reiserfs::resize( const Partition & partition_new, OperationDetail & operat
 bool reiserfs::check_repair( const Partition & partition, OperationDetail & operationdetail )
 {
        exit_status = execute_command( "reiserfsck --yes --fix-fixable --quiet " + partition.get_path(),
-                                      operationdetail, false, true );
-       
+                                      operationdetail, EXEC_CANCEL_SAFE );
+
        return ( exit_status == 0 || exit_status == 1 || exit_status == 256 ) ;
 }
 
diff --git a/src/xfs.cc b/src/xfs.cc
index 9c1cd8d..205af20 100644
--- a/src/xfs.cc
+++ b/src/xfs.cc
@@ -175,8 +175,7 @@ bool xfs::create( const Partition & new_partition, OperationDetail & operationde
        return ! execute_command( "mkfs.xfs -f -L \"" + new_partition.get_filesystem_label() + "\" " +
                                  new_partition.get_path(),
                                  operationdetail,
-                                 false,
-                                 true );
+                                 EXEC_CANCEL_SAFE );
 }
 
 bool xfs::resize( const Partition & partition_new, OperationDetail & operationdetail, bool fill_partition )
@@ -190,17 +189,17 @@ bool xfs::resize( const Partition & partition_new, OperationDetail & operationde
                if ( mount_point.empty() )
                        return false ;
                success &= ! execute_command( "mount -v -t xfs " + partition_new .get_path() + " " + 
mount_point,
-                                             operationdetail, true ) ;
+                                             operationdetail, EXEC_CHECK_STATUS );
        }
        else
                mount_point = partition_new .get_mountpoint() ;
 
        if ( success )
        {
-               success &= ! execute_command( "xfs_growfs " + mount_point, operationdetail, true ) ;
+               success &= ! execute_command( "xfs_growfs " + mount_point, operationdetail, EXEC_CHECK_STATUS 
);
 
                if ( ! partition_new .busy )
-                       success &= ! execute_command( "umount -v " + mount_point, operationdetail, true ) ;
+                       success &= ! execute_command( "umount -v " + mount_point, operationdetail, 
EXEC_CHECK_STATUS );
        }
 
        if ( ! partition_new .busy )
@@ -215,7 +214,8 @@ bool xfs::copy( const Partition & src_part,
 {
        bool success = true ;
 
-       success &= ! execute_command( "mkfs.xfs -f " + dest_part.get_path(), operationdetail, true, true );
+       success &= ! execute_command( "mkfs.xfs -f " + dest_part.get_path(), operationdetail,
+                                     EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE );
        if ( ! success )
                return false ;
 
@@ -231,23 +231,24 @@ bool xfs::copy( const Partition & src_part,
        }
 
        success &= ! execute_command( "mount -v -t xfs -o noatime,ro " + src_part.get_path() +
-                                     " " + src_mount_point, operationdetail, true ) ;
+                                     " " + src_mount_point, operationdetail, EXEC_CHECK_STATUS );
 
        if ( success )
        {
                success &= ! execute_command( "mount -v -t xfs " + dest_part.get_path() +
-                                             " " + dest_mount_point, operationdetail, true ) ;
+                                             " " + dest_mount_point, operationdetail, EXEC_CHECK_STATUS );
 
                if ( success )
                {
                        success &= ! execute_command( "sh -c 'xfsdump -J - " + src_mount_point +
-                                                     " | xfsrestore -J - " + dest_mount_point + "'",
-                                                     operationdetail, true, true );
+                                                     " | xfsrestore -J - " + dest_mount_point + "'",
+                                                     operationdetail, EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE );
 
-                       success &= ! execute_command( "umount -v " + dest_part.get_path(), operationdetail, 
true ) ;
+                       success &= ! execute_command( "umount -v " + dest_part.get_path(), operationdetail,
+                                                     EXEC_CHECK_STATUS );
                }
 
-               success &= ! execute_command( "umount -v " + src_part.get_path(), operationdetail, true ) ;
+               success &= ! execute_command( "umount -v " + src_part.get_path(), operationdetail, 
EXEC_CHECK_STATUS );
        }
 
        rm_temp_dir( dest_mount_point, operationdetail ) ;
@@ -260,9 +261,7 @@ bool xfs::copy( const Partition & src_part,
 bool xfs::check_repair( const Partition & partition, OperationDetail & operationdetail )
 {
        return ! execute_command( "xfs_repair -v " + partition .get_path(), operationdetail,
-                                 false, true );
+                                 EXEC_CANCEL_SAFE );
 }
 
 } //GParted
-
-
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]