[gparted] Refactor Win_GParted::unmount_partition() (#775932)

[Curtis Gedak <gedakc src gnome org>]

commit bfc16bd4a61b96f67703dbe225838c111b8a1999
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Sat Sep 10 19:34:10 2016 +0100

    Refactor Win_GParted::unmount_partition() (#775932)
    
    The primary reason to refactor unmount_partition() is to pass the
    Partition object to be unmounted, rather than use member variable
    selected_partition_ptr so that it doesn't have to handle the differences
    between encrypted and non-encrypted Partition objects.  The calling
    function can deal with that instead.  Then there were lots of small
    reasons to change almost every other line too:
    * Return success or failure rather than updating a passed pointer with
      the result.  Leftover from when the function used to be a separate
      thread:
          commit 52a2a9b00a32996921ace055e71d0e09fb33c5fe
          Reduce threading (#685740)
    * Pass updated error string by reference rather than by pointer.  Likely
      another leftover.
    * Stop borrowing the updated error string as a local variable for the
      error output from the umount command.  Use new umount_error local
      variable instead.  Was bad practice making the code harder to
      understand.
    * Rename failed_mountpoints to skipped_mountpoints to better reflect
      that it contains the mount points not attempted to be unmounted
      because two or more file systems are mounted at that point.
    * Rename errors to umount_errors to better reflect it contains the
      errors from each failed umount command.
    * Document the reason for mount points being skipped.
    * Update the skipped mount points message to state definitely why they
      could not be unmounted rather than stating most likely.
    * Simplify logic processing the error cases and return value.
    * Made static because it no longer accesses any class members.
    * Remove out dated "//threads.." comment from the header.  Another
      leftover from when the function use to be a separate thread.
    
    Bug 775932 - Refactor mostly applying of operations

 include/Win_GParted.h |    7 ++---
 src/Win_GParted.cc    |   64 ++++++++++++++++++++++++++++--------------------
 2 files changed, 40 insertions(+), 31 deletions(-)
---
diff --git a/include/Win_GParted.h b/include/Win_GParted.h
index 4cb9ef4..4bac079 100644
--- a/include/Win_GParted.h
+++ b/include/Win_GParted.h
@@ -141,10 +141,9 @@ private:
                static_cast<Gtk::CheckMenuItem *>( & menubar_main .items()[ 1 ] .get_submenu() ->items()[ 2 ] 
)
                        ->set_sensitive( state ) ; 
        }
-               
-       //threads..
-       void unmount_partition( bool * succes, Glib::ustring * error );
-               
+
+       static bool unmount_partition( const Partition & partition, Glib::ustring & error );
+
        //signal handlers
        void open_operationslist() ;
        void close_operationslist() ;
diff --git a/src/Win_GParted.cc b/src/Win_GParted.cc
index 5e12d8b..9d1377e 100644
--- a/src/Win_GParted.cc
+++ b/src/Win_GParted.cc
@@ -2221,40 +2221,50 @@ void Win_GParted::activate_format( GParted::FILESYSTEM new_fs )
        show_operationslist() ;
 }
 
-void Win_GParted::unmount_partition( bool * succes, Glib::ustring * error ) 
-{
-       std::vector<Glib::ustring> errors;
-       std::vector<Glib::ustring> failed_mountpoints;
-       std::vector<Glib::ustring> mountpoints = Mount_Info::get_all_mountpoints();
-       Glib::ustring dummy ;
-
-       *succes = true ; 
-       for ( unsigned int t = 0 ; t < selected_partition_ptr->get_mountpoints().size() ; t++ )
-               if ( std::count( mountpoints.begin(),
-                                mountpoints.end(),
-                                selected_partition_ptr->get_mountpoints()[t] ) <= 1 )
+bool Win_GParted::unmount_partition( const Partition & partition, Glib::ustring & error )
+{
+       const std::vector<Glib::ustring> fs_mountpoints = partition.get_mountpoints();
+       const std::vector<Glib::ustring> all_mountpoints = Mount_Info::get_all_mountpoints();
+
+       std::vector<Glib::ustring> skipped_mountpoints;
+       std::vector<Glib::ustring> umount_errors;
+
+       for ( unsigned int i = 0 ; i < fs_mountpoints.size() ; i ++ )
+       {
+               if ( std::count( all_mountpoints.begin(), all_mountpoints.end(), fs_mountpoints[i] ) >= 2 )
                {
-                       Glib::ustring cmd = "umount -v \"" + selected_partition_ptr->get_mountpoints()[t] + 
"\"";
-                       if ( Utils::execute_command( cmd, dummy, *error ) )
-                       {
-                               *succes = false ;
-                               errors.push_back( "# " + cmd + "\n" + *error );
-                       }
+                       // This mount point has two or more different file systems mounted
+                       // on top of each other.  Refuse to unmount it as in the general
+                       // case all have to be unmounted and then all except the file
+                       // system being unmounted have to be remounted.  This is too rare
+                       // a case to write such complicated code for.
+                       skipped_mountpoints.push_back( fs_mountpoints[i] );
                }
                else
-                       failed_mountpoints.push_back( selected_partition_ptr->get_mountpoints()[t] );
+               {
+                       Glib::ustring cmd = "umount -v \"" + fs_mountpoints[i] + "\"";
+                       Glib::ustring dummy;
+                       Glib::ustring umount_error;
+                       if ( Utils::execute_command( cmd, dummy, umount_error ) )
+                               umount_errors.push_back( "# " + cmd + "\n" + umount_error );
+               }
+       }
 
-       if ( *succes && failed_mountpoints .size() )
+       if ( umount_errors.size() > 0 )
        {
-               *succes = false ;
-               *error = _("The partition could not be unmounted from the following mount points:") ;
-               *error += "\n\n<i>" + Glib::build_path( "\n", failed_mountpoints ) + "</i>\n\n" ;
-               *error +=  _("Most likely other partitions are also mounted on these mount points. You are 
advised to unmount them manually.") ;
+               error = "<i>" + Glib::build_path( "</i>\n<i>", umount_errors ) + "</i>";
+               return false;
        }
-       else
-               *error = "<i>" + Glib::build_path( "\n", errors ) + "</i>" ;
+       if ( skipped_mountpoints.size() > 0 )
+       {
+               error = _("The partition could not be unmounted from the following mount points:");
+               error += "\n\n<i>" + Glib::build_path( "\n", skipped_mountpoints ) + "</i>\n\n";
+               error += _("This is because other partitions are also mounted on these mount points.  You are 
advised to unmount them manually.");
+               return false;
+       }
+       return true;
 }
-       
+
 void Win_GParted::toggle_busy_state()
 {
        g_assert( selected_partition_ptr != NULL );  // Bug: Partition callback without a selected partition
@@ -2370,7 +2380,7 @@ void Win_GParted::toggle_busy_state()
        else if ( selected_partition_ptr->busy )
        {
                show_pulsebar( String::ucompose( _("Unmounting %1"), selected_partition_ptr->get_path() ) );
-               unmount_partition( &success, &error );
+               success = unmount_partition( *selected_partition_ptr, error );
                hide_pulsebar();
                if ( ! success )
                {