From 2b57229fc21e6bd12ba6d50ea451691ec7408a11 Mon Sep 17 00:00:00 2001 From: Mike Fleetwood Date: Sun, 6 Sep 2015 14:39:07 +0100 Subject: [PATCH] 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 b0a9fd09..142e0b33 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 6bd0c551..2915f8fe 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 cb832b5a..b4f8f712 100644 --- a/src/Utils.cc +++ b/src/Utils.cc @@ -33,6 +33,8 @@ #include #include #include +#include +#include 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 522f7e47..805472e2 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 fb96c162..7405d656 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 1782e71a..e203422d 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 67680ead..d48b5a05 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 536498d8..9b1cc21d 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; }