From 438b35aed9aabc9f0e634831764df2b632129f69 Mon Sep 17 00:00:00 2001 From: Mike Fleetwood Date: Wed, 3 Feb 2016 21:17:30 +0000 Subject: [PATCH] Connect timed progress tracking callbacks inside execute_command() (#760709) The timed progress tracking callback for execution of xfs copy follows this pattern: sigc::connection c; ... c = Glib::signal_timeout().connect( ... sigc::mem_fun( *this, &xfs::copy_progress ) ..., 500 /*ms*/ ); ... execute_command( ... ); c.disconnect(); As with output progress tracking callbacks for ext2/3/4 and ntfs file system specific commands, pass the callback slot and a flag into execute_command() and connect the timed callback inside. This simplified the pattern to: ... execute_command( ...|EXEC_PROGRESS_TIMED, static_cast( sigc::mem_fun( *this, &xfs::copy_progress ) ) ); NOTE: The type of sigc::mem_fun() doesn't allow the compiler to choose between the two overloaded variants of execute_command() with the fourth parameter of either (full types without typedefs of StreamSlot and TimedSlot respectively): sigc::slot stream_progress_slot sigc::slot timed_progress_slot Therefore have to cast the result of all callback slots to the relevant type. Hence: static_cast( sigc::mem_fun( *this, &{CLASS}::{NAME}_progress ) ) static_cast( sigc::mem_fun( *this, &xfs::copy_progress ) ) References: * [sigc] Functor not resolving between overloaded methods with different slot types https://mail.gnome.org/archives/libsigc-list/2016-February/msg00000.html * Bug 306705 - Can't overload methods based on different slot<> parameters. https://bugzilla.gnome.org/show_bug.cgi?id=306705 Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific copy methods --- include/FileSystem.h | 24 ++++++++++++++---------- src/FileSystem.cc | 34 +++++++++++++++++++++++++++++++++- src/ext2.cc | 10 +++++----- src/ntfs.cc | 2 +- src/xfs.cc | 19 +++++++++---------- 5 files changed, 62 insertions(+), 27 deletions(-) diff --git a/include/FileSystem.h b/include/FileSystem.h index 71510553..c5693bd4 100644 --- a/include/FileSystem.h +++ b/include/FileSystem.h @@ -41,7 +41,8 @@ enum ExecFlags EXEC_CANCEL_SAFE = 1 << 2, EXEC_PROGRESS_STDOUT = 1 << 3, // Run progress tracking callback after reading new // data on stdout from command. - EXEC_PROGRESS_STDERR = 1 << 4 // Same but for stderr. + EXEC_PROGRESS_STDERR = 1 << 4, // Same but for stderr. + EXEC_PROGRESS_TIMED = 1 << 5 // Run progress tracking callback periodically. }; inline ExecFlags operator|( ExecFlags lhs, ExecFlags rhs ) @@ -82,17 +83,16 @@ public: protected: typedef sigc::slot StreamSlot; + typedef sigc::slot TimedSlot; - // Use sigc::slot<> class default constructor, via StreamSlot typedef, to create - // an empty, unconnected slot to use as the default stream_progress_slot parameter - // for when none is provided. - // References: - // * How to set default parameter as class object in c++? - // http://stackoverflow.com/questions/12121645/how-to-set-default-parameter-as-class-object-in-c - // * C++ function, what default value can I give for an object? - // http://stackoverflow.com/questions/9909444/c-function-what-default-value-can-i-give-for-an-object int execute_command( const Glib::ustring & command, OperationDetail & operationdetail, - ExecFlags flags = EXEC_NONE, StreamSlot stream_progress_slot = StreamSlot() ); + ExecFlags flags = EXEC_NONE ); + int execute_command( const Glib::ustring & command, OperationDetail & operationdetail, + ExecFlags flags, + StreamSlot stream_progress_slot ); + int execute_command( const Glib::ustring & command, OperationDetail & operationdetail, + ExecFlags flags, + TimedSlot timed_progress_slot ); void set_status( OperationDetail & operationdetail, bool success ); void execute_command_eof(); Glib::ustring mk_temp_dir( const Glib::ustring & infix, OperationDetail & operationdetail ) ; @@ -105,6 +105,10 @@ protected: unsigned int index ; private: + int execute_command_internal( const Glib::ustring & command, OperationDetail & operationdetail, + ExecFlags flags, + StreamSlot stream_progress_slot, + TimedSlot timed_progress_slot ); void store_exit_status( GPid pid, int status ); bool running; int pipecount; diff --git a/src/FileSystem.cc b/src/FileSystem.cc index e83bbd74..1cd6a1d6 100644 --- a/src/FileSystem.cc +++ b/src/FileSystem.cc @@ -81,7 +81,33 @@ static void setup_child() } int FileSystem::execute_command( const Glib::ustring & command, OperationDetail & operationdetail, - ExecFlags flags, StreamSlot stream_progress_slot ) + ExecFlags flags ) +{ + StreamSlot empty_stream_slot; + TimedSlot empty_timed_slot; + return execute_command_internal( command, operationdetail, flags, empty_stream_slot, empty_timed_slot ); +} + +int FileSystem::execute_command( const Glib::ustring & command, OperationDetail & operationdetail, + ExecFlags flags, + StreamSlot stream_progress_slot ) +{ + TimedSlot empty_timed_slot; + return execute_command_internal( command, operationdetail, flags, stream_progress_slot, empty_timed_slot ); +} + +int FileSystem::execute_command( const Glib::ustring & command, OperationDetail & operationdetail, + ExecFlags flags, + TimedSlot timed_progress_slot ) +{ + StreamSlot empty_stream_slot; + return execute_command_internal( command, operationdetail, flags, empty_stream_slot, timed_progress_slot ); +} + +int FileSystem::execute_command_internal( const Glib::ustring & command, OperationDetail & operationdetail, + ExecFlags flags, + StreamSlot stream_progress_slot, + TimedSlot timed_progress_slot ) { operationdetail.add_child( OperationDetail( command, STATUS_EXECUTE, FONT_BOLD_ITALIC ) ); Glib::Pid pid; @@ -126,12 +152,16 @@ int FileSystem::execute_command( const Glib::ustring & command, OperationDetail errorcapture.signal_update.connect( sigc::bind( sigc::ptr_fun( update_command_output ), children[children.size() - 1], &error ) ); + sigc::connection timed_conn; if ( flags & EXEC_PROGRESS_STDOUT && ! stream_progress_slot.empty() ) // Call progress tracking callback when stdout updates outputcapture.signal_update.connect( sigc::bind( stream_progress_slot, &operationdetail ) ); else if ( flags & EXEC_PROGRESS_STDERR && ! stream_progress_slot.empty() ) // Call progress tracking callback when stderr updates errorcapture.signal_update.connect( sigc::bind( stream_progress_slot, &operationdetail ) ); + else if ( flags & EXEC_PROGRESS_TIMED && ! timed_progress_slot.empty() ) + // Call progress tracking callback every 500 ms + timed_conn = Glib::signal_timeout().connect( sigc::bind( timed_progress_slot, &operationdetail ), 500 ); outputcapture.connect_signal(); errorcapture.connect_signal(); @@ -151,6 +181,8 @@ int FileSystem::execute_command( const Glib::ustring & command, OperationDetail } close( out ); close( err ); + if ( timed_conn.connected() ) + timed_conn.disconnect(); operationdetail.stop_progressbar(); return exit_status; } diff --git a/src/ext2.cc b/src/ext2.cc index cb2b873f..bcd05309 100644 --- a/src/ext2.cc +++ b/src/ext2.cc @@ -231,7 +231,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, EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE|EXEC_PROGRESS_STDOUT, - sigc::mem_fun( *this, &ext2::create_progress ) ); + static_cast( sigc::mem_fun( *this, &ext2::create_progress ) ) ); } bool ext2::resize( const Partition & partition_new, OperationDetail & operationdetail, bool fill_partition ) @@ -243,14 +243,14 @@ bool ext2::resize( const Partition & partition_new, OperationDetail & operationd partition_new .get_sector_length(), partition_new .sector_size, UNIT_KIB ) ) ) + "K"; return ! execute_command( str_temp, operationdetail, EXEC_CHECK_STATUS|EXEC_PROGRESS_STDOUT, - sigc::mem_fun( *this, &ext2::resize_progress ) ); + static_cast( sigc::mem_fun( *this, &ext2::resize_progress ) ) ); } bool ext2::check_repair( const Partition & partition, OperationDetail & operationdetail ) { exit_status = execute_command( fsck_cmd + " -f -y -v -C 0 " + partition.get_path(), operationdetail, EXEC_CANCEL_SAFE|EXEC_PROGRESS_STDOUT, - sigc::mem_fun( *this, &ext2::check_repair_progress ) ); + static_cast( sigc::mem_fun( *this, &ext2::check_repair_progress ) ) ); bool success = ( exit_status == 0 || exit_status == 1 || exit_status == 2 ); set_status( operationdetail, success ); return success; @@ -270,7 +270,7 @@ bool ext2::move( const Partition & partition_new, fs_block_size = partition_old.fs_block_size; return ! execute_command( cmd, operationdetail, EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE|EXEC_PROGRESS_STDERR, - sigc::mem_fun( *this, &ext2::copy_progress ) ); + static_cast( sigc::mem_fun( *this, &ext2::copy_progress ) ) ); } bool ext2::copy( const Partition & src_part, @@ -280,7 +280,7 @@ bool ext2::copy( const Partition & src_part, fs_block_size = src_part.fs_block_size; return ! execute_command( image_cmd + " -ra -p " + src_part.get_path() + " " + dest_part.get_path(), operationdetail, EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE|EXEC_PROGRESS_STDERR, - sigc::mem_fun( *this, &ext2::copy_progress ) ); + static_cast( sigc::mem_fun( *this, &ext2::copy_progress ) ) ); } //Private methods diff --git a/src/ntfs.cc b/src/ntfs.cc index d5d67005..cf50ee3a 100644 --- a/src/ntfs.cc +++ b/src/ntfs.cc @@ -236,7 +236,7 @@ bool ntfs::resize( const Partition & partition_new, OperationDetail & operationd if ( ! execute_command( cmd + " " + partition_new.get_path(), operationdetail.get_last_child(), EXEC_CHECK_STATUS|EXEC_PROGRESS_STDOUT, - sigc::mem_fun( *this, &ntfs::resize_progress ) ) ) + static_cast( sigc::mem_fun( *this, &ntfs::resize_progress ) ) ) ) { operationdetail .get_last_child() .set_status( STATUS_SUCCES ) ; return_value = true ; diff --git a/src/xfs.cc b/src/xfs.cc index ccc7b52f..d9fa0209 100644 --- a/src/xfs.cc +++ b/src/xfs.cc @@ -254,20 +254,13 @@ bool xfs::copy( const Partition & src_part, if ( success ) { - sigc::connection c; if ( src_used > 0LL ) - { operationdetail.run_progressbar( 0.0, (double)src_used, PROGRESSBAR_TEXT_COPY_BYTES ); - // Get xfs::copy_progress() called every 500 ms to update progress - c = Glib::signal_timeout().connect( - sigc::bind( sigc::mem_fun( *this, &xfs::copy_progress ), - &operationdetail ), - 500 ); - } success &= ! execute_command( "sh -c 'xfsdump -J - " + src_mount_point + " | xfsrestore -J - " + dest_mount_point + "'", - operationdetail, EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE ); - c.disconnect(); + operationdetail, + EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE|EXEC_PROGRESS_TIMED, + static_cast( sigc::mem_fun( *this, &xfs::copy_progress ) ) ); operationdetail.stop_progressbar(); success &= ! execute_command( "umount -v " + dest_part.get_path(), operationdetail, @@ -296,6 +289,12 @@ bool xfs::check_repair( const Partition & partition, OperationDetail & operation // recorded source FS used bytes. bool xfs::copy_progress( OperationDetail * operationdetail ) { + if ( src_used <= 0LL ) + { + operationdetail->stop_progressbar(); + // Failed to get source FS used bytes. Remove this timed callback early. + return false; + } Byte_Value fs_size; Byte_Value fs_free; Byte_Value dst_used;