[gparted] Implement shell style exit status decoding (#754684)



commit 2b57229fc21e6bd12ba6d50ea451691ec7408a11
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Sun Sep 6 14:39:07 2015 +0100

    Implement shell style exit status decoding (#754684)
    
    Command exit status is a 1 byte value between 0 and 255. [1][2]  However
    at the Unix API level the value is encoded as documented in the
    waitpid(2) manual page.  This is true for the Glib API too. [3]  This is
    why, for example, the comment in ext2::check_repair() reported receiving
    undocumented exit status 256.  It was actually receiving exit status 1
    encoded as per the waitpid(2) method.
    
    Add shell style exit status decoding [2] to execution of all external
    commands.   Return value from Utils::execute_command() and
    FileSystem::execute_command() functions are now:
        0 - 125 - Exit status from the command
        126     - Error executing the command
        127     - Command not found
        128+N   - Command terminated by signal N
        255     - Unexpected waitpid(2) condition
    Also adjust checking of the returned statuses as necessary.
    
    [1] Advanced Bash-Scripting Guide: Appendix D. Exit Codes With Special
        Meanings
        http://www.linuxtopia.org/online_books/advanced_bash_scripting_guide/exitcodes.html
    
    [2] Quote from the bash(1) manual page:
    
            EXIT STATUS
                ... Exit statuses fall between 0 and 255, though as
                explained below, the shell may use values above 125
                specially.  ...
    
                ... When a command terminates on a fatal signal N, bash uses
                the value of 128+N as the exit status.
    
                If a command is not found, the child process created to
                execute it returns a status of 127.  If a command is found
                but is not executable, the return status is 126.
    
    [3] Quote from the Glib Reference Manual, Spawning Processes section,
        for function g_spawn_check_exit_status():
        https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#g-spawn-check-exit-status
    
            The g_spawn_sync() and g_child_watch_add() family of APIs return
            an exit status for subprocesses encoded in a platform-specific
            way.  On Unix, this is guaranteed to be in the same format
            waitpid() returns, ...
    
    Bug 754684 - Updates to FileSystem:: and Utils::execute_command()
                 functions

 include/Utils.h   |    2 ++
 src/FileSystem.cc |    4 ++--
 src/Utils.cc      |   34 ++++++++++++++++++++++++++++++++--
 src/btrfs.cc      |    4 ++--
 src/ext2.cc       |    5 +----
 src/fat16.cc      |    4 ++--
 src/ntfs.cc       |    2 +-
 src/reiserfs.cc   |    9 +++++++--
 8 files changed, 49 insertions(+), 15 deletions(-)
---
diff --git a/include/Utils.h b/include/Utils.h
index b0a9fd0..142e0b3 100644
--- a/include/Utils.h
+++ b/include/Utils.h
@@ -190,6 +190,8 @@ public:
                                    Glib::ustring & output,
                                    Glib::ustring & error,
                                    bool use_C_locale = false ) ;
+       static int get_failure_status( Glib::SpawnError & e );
+       static int decode_wait_status( int wait_status );
        static Glib::ustring regexp_label( const Glib::ustring & text
                                         , const Glib::ustring & pattern
                                         ) ;
diff --git a/src/FileSystem.cc b/src/FileSystem.cc
index 6bd0c55..2915f8f 100644
--- a/src/FileSystem.cc
+++ b/src/FileSystem.cc
@@ -54,7 +54,7 @@ const Glib::ustring FileSystem::get_generic_text( CUSTOM_TEXT ttype, int index )
 
 void FileSystem::store_exit_status( GPid pid, int status )
 {
-       exit_status = status;
+       exit_status = Utils::decode_wait_status( status );
        running = false;
        if (pipecount == 0) // pipes finished first
                Gtk::Main::quit();
@@ -102,7 +102,7 @@ int FileSystem::execute_command( const Glib::ustring & command, OperationDetail
                std::cerr << e.what() << std::endl;
                operationdetail.get_last_child().add_child(
                        OperationDetail( e.what(), STATUS_ERROR, FONT_ITALIC ) );
-               return 1;
+               return Utils::get_failure_status( e );
        }
        fcntl( out, F_SETFL, O_NONBLOCK );
        fcntl( err, F_SETFL, O_NONBLOCK );
diff --git a/src/Utils.cc b/src/Utils.cc
index cb832b5..b4f8f71 100644
--- a/src/Utils.cc
+++ b/src/Utils.cc
@@ -33,6 +33,8 @@
 #include <sys/statvfs.h>
 #include <gtkmm/main.h>
 #include <fcntl.h>
+#include <sys/types.h>
+#include <sys/wait.h>
 
 namespace GParted
 {
@@ -529,7 +531,7 @@ public:
 
 void CommandStatus::store_exit_status( GPid pid, int status )
 {
-       exit_status = status;
+       exit_status = Utils::decode_wait_status( status );
        running = false;
        if (pipecount == 0) // pipes finished first
        {
@@ -596,7 +598,7 @@ int Utils::execute_command( const Glib::ustring & command,
                        &err );
        } catch (Glib::SpawnError &e) {
                std::cerr << e.what() << std::endl;
-               return 1;
+               return Utils::get_failure_status( e );
        }
        fcntl( out, F_SETFL, O_NONBLOCK );
        fcntl( err, F_SETFL, O_NONBLOCK );
@@ -625,6 +627,34 @@ int Utils::execute_command( const Glib::ustring & command,
        return status.exit_status;
 }
 
+// Return shell style exit status when failing to execute a command.  127 for command not
+// found and 126 otherwise.
+// NOTE:
+// Together get_failure_status() and decode_wait_status() provide complete shell style
+// exit status handling.  See bash(1) manual page, EXIT STATUS section for details.
+int Utils::get_failure_status( Glib::SpawnError & e )
+{
+       if ( e.code() == Glib::SpawnError::NOENT )
+               return 127;
+       return 126;
+}
+
+// Return shell style decoding of waitpid(2) encoded exit statuses.  Return command exit
+// status or 128 + signal number when terminated by a signal.
+int Utils::decode_wait_status( int wait_status )
+{
+       if ( WIFEXITED( wait_status ) )
+               return WEXITSTATUS( wait_status );
+       else if ( WIFSIGNALED( wait_status ) )
+               return 128 + WTERMSIG( wait_status );
+
+       // Other cases of WIFSTOPPED() and WIFCONTINUED() occur when the process is
+       // stopped or resumed by signals.  Should be impossible as this function is only
+       // called after the process has exited.
+       std::cerr << "Unexpected wait status " << wait_status << std::endl;
+       return 255;
+}
+
 Glib::ustring Utils::regexp_label( const Glib::ustring & text
                                  , const Glib::ustring & pattern
                                  )
diff --git a/src/btrfs.cc b/src/btrfs.cc
index 522f7e4..805472e 100644
--- a/src/btrfs.cc
+++ b/src/btrfs.cc
@@ -356,8 +356,8 @@ bool btrfs::resize( const Partition & partition_new, OperationDetail & operation
                        //  but not ignoring them will cause resizing to the
                        //  same size as part of check operation to fail.
                        resize_succeeded = (    exit_status == 0
-                                            || (   btrfs_found && exit_status == 30<<8 )
-                                            || ( ! btrfs_found && exit_status ==  1<<8 )
+                                            || (   btrfs_found && exit_status == 30 )
+                                            || ( ! btrfs_found && exit_status ==  1 )
                                           ) ;
                }
                set_status( operationdetail, resize_succeeded );
diff --git a/src/ext2.cc b/src/ext2.cc
index fb96c16..7405d65 100644
--- a/src/ext2.cc
+++ b/src/ext2.cc
@@ -240,10 +240,7 @@ bool ext2::check_repair( const Partition & partition, OperationDetail & operatio
 {
        exit_status = execute_command( fsck_cmd + " -f -y -v -C 0 " + partition.get_path(), operationdetail,
                                       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...
-       bool success = ( exit_status == 0 || exit_status == 1 || exit_status == 2 || exit_status == 256 );
+       bool success = ( exit_status == 0 || exit_status == 1 || exit_status == 2 );
        set_status( operationdetail, success );
        return success;
 }
diff --git a/src/fat16.cc b/src/fat16.cc
index 1782e71..e203422 100644
--- a/src/fat16.cc
+++ b/src/fat16.cc
@@ -128,7 +128,7 @@ FS fat16::get_filesystem_support()
 void fat16::set_used_sectors( Partition & partition ) 
 {
        exit_status = Utils::execute_command( check_cmd + " -n -v " + partition .get_path(), output, error, 
true ) ;
-       if ( exit_status == 0 || exit_status == 1 || exit_status == 256 )
+       if ( exit_status == 0 || exit_status == 1 )
        {
                //total file system size in logical sectors
                index = output .rfind( "\n", output .find( "sectors total" ) ) +1 ;
@@ -239,7 +239,7 @@ bool fat16::check_repair( const Partition & partition, OperationDetail & operati
 {
        exit_status = execute_command( check_cmd + " -a -w -v " + partition .get_path(), operationdetail,
                                       EXEC_CANCEL_SAFE );
-       bool success = ( exit_status == 0 || exit_status == 1 || exit_status == 256 );
+       bool success = ( exit_status == 0 || exit_status == 1 );
        set_status( operationdetail, success );
        return success;
 }
diff --git a/src/ntfs.cc b/src/ntfs.cc
index 67680ea..d48b5a0 100644
--- a/src/ntfs.cc
+++ b/src/ntfs.cc
@@ -121,7 +121,7 @@ void ntfs::set_used_sectors( Partition & partition )
 {
        exit_status = Utils::execute_command(
                "ntfsresize --info --force --no-progress-bar " + partition .get_path(), output, error, true ) 
;
-       if ( exit_status == 0 || exit_status == 1<<8 )
+       if ( exit_status == 0 || exit_status == 1 )
        {
                index = output .find( "Current volume size:" ) ;
                if ( index >= output .length() ||
diff --git a/src/reiserfs.cc b/src/reiserfs.cc
index 536498d..9b1cc21 100644
--- a/src/reiserfs.cc
+++ b/src/reiserfs.cc
@@ -176,7 +176,12 @@ bool reiserfs::resize( const Partition & partition_new, OperationDetail & operat
        Glib::ustring cmd = "sh -c 'echo y | resize_reiserfs" + size + " " + partition_new .get_path() + "'" ;
 
        exit_status = execute_command( cmd, operationdetail ) ;
-       bool success = ( exit_status == 0 || exit_status == 256 );
+       // NOTE: Neither resize_reiserfs manual page nor the following commit, which first
+       // added this check, indicate why exit status 1 also indicates success.  Commit
+       // from 2006-05-23:
+       //     7bb7e8a84f164cd913384509a6adc3739a9d8b78
+       //     Use ped_device_read and ped_device_write instead of 'dd' to copy
+       bool success = ( exit_status == 0 || exit_status == 1 );
        set_status( operationdetail, success );
        return success;
 }
@@ -185,7 +190,7 @@ bool reiserfs::check_repair( const Partition & partition, OperationDetail & oper
 {
        exit_status = execute_command( "reiserfsck --yes --fix-fixable --quiet " + partition.get_path(),
                                       operationdetail, EXEC_CANCEL_SAFE );
-       bool success = ( exit_status == 0 || exit_status == 1 || exit_status == 256 );
+       bool success = ( exit_status == 0 || exit_status == 1 );
        set_status( operationdetail, success );
        return success;
 }


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