[gparted] Remove coding landmine in get_disk() (#152)

[Curtis Gedak <gedakc src gnome org>]

commit c4ef43aa5d551569ca5cad5912d51f0f9df261d2
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Sat Apr 10 16:39:22 2021 +0100

    Remove coding landmine in get_disk() (#152)
    
    get_disk() is the wrapper around libparted's ped_disk_new() which reads
    a disk label from the specified device and if successful creates the in
    memory PedDisk object to represent it.  In the case that libparted
    doesn't recognise a disk label or a file system, having get_disk() go
    and destroy the passed in PedDevice object via parameter lp_device is
    very unexpected behaviour hence describing it as a coding landmine.
    
    BACKGROUND
    
    1. Early on GParted only worked with devices with valid disk labels.
       FileSystem.h:open_device_and_disk() required both ped_device_get()
       and ped_disk_new() to succeed or neither to succeed.
    2. Commit [1] added support for devices which didn't yet have a disk
       label.  open_device_and_disk() had default parameter strict=true
       added.  While scanning strict=false was passed which allowed
       open_device_and_disk() to return success if only ped_device_get()
       succeeded and ped_disk_new() failed when the disk was empty.  All
       other times open_device_and_disk() was called with default
       strict=true, still requiring both or neither to succeed.
    3. Commit [2] added support for whole disk file systems.  The now named
       get_device_and_disk() had it's functionality split between
       get_device() and get_disk().  This result in the code landmine being
       left behind: get_disk() destroying the passed device object if
       default parameter strict=true and no disk label or file system was
       detected.
    
    ANALYSIS
    
    1. Since support for whole disk file systems [2] all current calls to
       get_device_and_disk() let the strict parameter default to true and
       are only called on known partitions within disk labels when applying
       a change to that partition.  Therefore they don't care about the
       behaviour of get_disk(), just that get_device_and_disk() maintains
       that both ped_device_get() and ped_disk_new() succeed or neither
       succeed.
    2. Two direct calls to get_disk() where the strict parameter defaults to
       true, from calibrate_partition() and erase_filesystem_signatures(),
       only do so on known partitions within disk labels as part of applying
       a change to that partition.  Therefore ped_disk_new() will succeed
       and so PedDevice isn't deleted when not wanted.
    3. The two remaining direct calls to get_disk() where the strict
       parameter is explicitly set to false, from set_device_from_disk() and
       detect_filesystem_in_encryption_mapping(), are when scanning.  As the
       pass strict=false they don't allow the PedDevice deletion to occur if
       no recognised disk label is found.
    
    FIX
    
    Remove the strict parameter from get_disk() and get_device_and_disk() as
    it's no longer needed.  Remove the code landmine by removing the side
    affect of destroying the PedDevice object if a disk label isn't found.
    Make sure get_device_and_disk() maintains the all or nothing behaviour.
    
    Also don't pass lp_device by reference to a pointer to get_disk() so the
    code can't change where lp_device points.
    
    [1] 038c5c5d99ad842f1a57f12222c884be29f4df4f
        P (special thanks to mantiena-baltix for bringing this issue to my
    
    [2] 51ac4d56489653854cd22787238a14bfa66a6ad4
        Split get_device_and_disk() into two (#743181)
    
    Closes #152 - GParted crashed when trying to probe an encrypted
                  partition containing content that libparted doesn't
                  recognise

 include/GParted_Core.h |  6 +++---
 src/GParted_Core.cc    | 43 ++++++++++++++++++++-----------------------
 2 files changed, 23 insertions(+), 26 deletions(-)
---
diff --git a/include/GParted_Core.h b/include/GParted_Core.h
index 0db4aa73..904b0923 100644
--- a/include/GParted_Core.h
+++ b/include/GParted_Core.h
@@ -222,9 +222,9 @@ private:
 
        static bool flush_device( PedDevice * lp_device );
        static bool get_device( const Glib::ustring & device_path, PedDevice *& lp_device, bool flush = false 
);
-       static bool get_disk( PedDevice *& lp_device, PedDisk *& lp_disk, bool strict = true );
-       static bool get_device_and_disk( const Glib::ustring & device_path,
-                                        PedDevice*& lp_device, PedDisk*& lp_disk, bool strict = true, bool 
flush = false );
+       static bool get_disk(PedDevice *lp_device, PedDisk*& lp_disk);
+       static bool get_device_and_disk(const Glib::ustring& device_path,
+                                       PedDevice*& lp_device, PedDisk*& lp_disk, bool flush = false);
        static void destroy_device_and_disk( PedDevice*& lp_device, PedDisk*& lp_disk );
        static bool commit( PedDisk* lp_disk );
        static bool commit_to_os( PedDisk* lp_disk, std::time_t timeout );
diff --git a/src/GParted_Core.cc b/src/GParted_Core.cc
index 6d6885ac..b6a45644 100644
--- a/src/GParted_Core.cc
+++ b/src/GParted_Core.cc
@@ -687,7 +687,7 @@ void GParted_Core::set_device_from_disk( Device & device, const Glib::ustring &
                        set_device_one_partition( device, lp_device, fstype, messages );
                }
                // Partitioned drive
-               else if ( get_disk( lp_device, lp_disk, false ) )
+               else if (get_disk(lp_device, lp_disk))
                {
                        // Partitioned drive (excluding "loop"), as recognised by libparted
                        if ( lp_disk && lp_disk->type && lp_disk->type->name &&
@@ -1089,7 +1089,7 @@ FSType GParted_Core::detect_filesystem_in_encryption_mapping(const Glib::ustring
                // supports one block device to one encryption mapping to one file system.
                PedDisk *lp_disk = NULL;
                PedPartition *lp_partition = NULL;
-               if (get_disk(lp_device, lp_disk, false) && lp_disk && lp_disk->type &&
+               if (get_disk(lp_device, lp_disk) && lp_disk && lp_disk->type        &&
                    lp_disk->type->name && strcmp(lp_disk->type->name, "loop") == 0   )
                {
                        lp_partition = ped_disk_next_partition(lp_disk, NULL);
@@ -4104,41 +4104,38 @@ bool GParted_Core::get_device( const Glib::ustring & device_path, PedDevice *& l
        return false;
 }
 
-bool GParted_Core::get_disk( PedDevice *& lp_device, PedDisk *& lp_disk, bool strict )
+
+bool GParted_Core::get_disk(PedDevice *lp_device, PedDisk*& lp_disk)
 {
-       if ( lp_device )
-       {
-               lp_disk = ped_disk_new( lp_device );
+       g_assert(lp_device != NULL);  // Bug: Not initialised by call to ped_device_get() or 
ped_device_get_next()
 
-               // (#762941)(!46) After ped_disk_new() wait for triggered udev rules to
-               // to complete which remove and re-add all the partition specific /dev
-               // entries to avoid FS specific commands failing because they happen to
-               // be running when the needed /dev/PTN entries don't exist.
-               settle_device(SETTLE_DEVICE_PROBE_MAX_WAIT_SECONDS);
+       lp_disk = ped_disk_new(lp_device);
 
-               // if ! disk and writable it's probably a HD without disklabel.
-               // We return true here and deal with them in
-               // GParted_Core::set_device_from_disk().
-               if ( lp_disk || ( ! strict && ! lp_device ->read_only ) )
-                       return true;
+       // (#762941)(!46) After ped_disk_new() wait for triggered udev rules to complete
+       // which remove and re-add all the partition specific /dev entries to avoid FS
+       // specific commands failing because they happen to be running when the needed
+       // /dev/PTN entries don't exist.
+       settle_device(SETTLE_DEVICE_PROBE_MAX_WAIT_SECONDS);
 
-               destroy_device_and_disk( lp_device, lp_disk );
-       }
-
-       return false;
+       return (lp_disk != NULL);
 }
 
-bool GParted_Core::get_device_and_disk( const Glib::ustring & device_path,
-                                        PedDevice*& lp_device, PedDisk*& lp_disk, bool strict, bool flush )
+
+bool GParted_Core::get_device_and_disk(const Glib::ustring& device_path,
+                                       PedDevice*& lp_device, PedDisk*& lp_disk, bool flush)
 {
        if ( get_device( device_path, lp_device, flush ) )
        {
-               return get_disk( lp_device, lp_disk, strict );
+               if (get_disk(lp_device, lp_disk))
+                       return true;
+
+               destroy_device_and_disk(lp_device, lp_disk);
        }
 
        return false;
 }
 
+
 void GParted_Core::destroy_device_and_disk( PedDevice*& lp_device, PedDisk*& lp_disk )
 {
        if ( lp_disk )