Now Win_GParted::m_display_device.partitions is an identical copy of
Win_GParted::display_partitions with the same lifetime. That's wasteful
and pointless. Therefore remove the later and use the former in it's
place.
Closes#227 - Unable to allocate 1 MiB between partitions when moving to
the right
The Create New and Paste dialogs also create partitions and have to
honour currently composed partitions while doing so. Therefore they
must have a Device object containing the currently composed partition
layout for passing into snap_to_alignment() and below. So copy the
current Device object when refreshing the visual at the same time
visual_partitions is generated and use in all 3 dialogs which compose
new partitions.
Note that Create New and Paste aren't subject to the same bug as Resize/
Move was because the code in snap_to_mebibyte() [1] checked the
partition object being composed has status STAT_REAL. This is true for
partition objects created by the Resize/Move dialog, but not true for
the Create New and Paste dialogs which set status to STAT_NEW and
STAT_COPY respectively instead.
[1] Dialog_Base_Partition::snap_to_mebibyte() lines 418 to 438
https://gitlab.gnome.org/GNOME/gparted/-/blob/GPARTED_1_5_0/src/Dialog_Base_Partition.cc#L418Closes#227 - Unable to allocate 1 MiB between partitions when moving to
the right
A number of GParted methods named get_*() are returning properties and
are return-by-value. For objects this implies the returned value is
copy constructed and later destroyed when it goes out of scope. Change
them to use return-by-constant-reference to avoid unnecessary object
duplication. Just have to make sure the reference goes out of scope
before the referenced object is destroyed to avoid having a reference
(pointer) to freed memory. Assigning to a local variable instead of
of a local reference still duplicates the object so can be used when the
lifetime of the gotten object needs to be longer that the referenced
object.
Previously done for other getters here:
d948cbcb91
Make get_custom_text() and get_generic_text() return by reference
This change just makes Device::get_path() return a constant reference
and updates uses to match to avoid copy constructing the returned value.
Closes!94 - Make more getter methods use return-by-constant-reference
When composing a resize operation on an open encryption mapping, use the
existing LUKS password dialog to prompt for the passphrase, if and only
if 'cryptsetup resize' will prompt and GParted doesn't already have a
password. 'cryptsetup resize' will prompt for a LUKS passphrase when
the passphrase was stored in the kernel keyring service,
key_loc == KEYLOC_KeyRing. See the previous commit "Capture LUKS
mapping master encryption key location (#59)" for more details.
As commented in the code GParted specifically doesn't support the case
where the LUKS passphrase is changed while GParted is running and it
knew the old passphrase. When resizing an open encryption mapping
GParted will just pass the old out of date passphrase it knows and the
resize will fail like this:
# cryptsetup status sdb2_crypt | egrep 'type|key location'
type: LUKS2
key location: keyring
# dmsetup table --target crypt
sdb2_crypt: 0 491520 crypt aes-xts-plain64 :64:logon:cryptsetup:3d040240-97ba-4559-af98-72c3be500498-d0 0 8:18 32768
# echo -n oldpassword | cryptsetup -v --size 491520 resize sdb2_crypt
No key available with this passphrase.
Command failed with code -2 (no permission or bad passphrase).
# echo $?
2
To work around this either close and restart GParted or close and reopen
the encryption mapping. The former because GParted doesn't save
passwords across a restart so will prompt and the latter because GParted
will use the wrong old passphrase to try to open the mapping and then
prompt for the correct passphrase until successfully opened.
Closes#59 - Resize of LUKS2 encrypted file system fails with "Nothing
to read on input"
Seems to be referring to how Fill_Label_Device_Info() worked in the
past, but from history before the beginning of the GIT repository.
Remove out of date comment.
To match the renaming performed as part of Bug 741424 "Add support for
GPT partition names". In particular this is most closely similar to:
d480800600
Rename enum to OPERATION_LABEL_FILESYSTEM (#741424)
Gtk::HBox and Gtk::VBox were deprecated in Gtkmm 3.2 [1]. Replace with
plain Gtk::Box.
This commit makes the change for Win_GParted.{h,cc}.
[1] Gtkmm 3.2.0 NEWS file (actually first included in gtkmm 3.1.6
unstable)
https://gitlab.gnome.org/GNOME/gtkmm/blob/3.2.0/NEWS#L91
Gtk:
* All H* or V* specialized classes have been deprecated, to
match the deprecations in the GTK+ C API. You should now
set the orientation instead.
This includes HBox, VBox, HButtonBox, VButtonBox, HPaned,
VPaned, HScale, VScale, HSeparator, VSeparator, HScrollbar and
VScrollbar.
Closes!25 - Modern Gtk3 - part 1
It is not creating a dialog (a pop-up window managed by GParted code
itself). It is launching independent yelp program to display the help,
so remove the "_dialog" from the name to avoid any possible confusion.
In Gtk3 the progress bar height is fixed and defined by the CSS theme in
use. Changing the widget allocation size does nothing, it is always
rendered the same way.
In many themes, including Adwaita, the progressbar is very, very thin.
Provide custom CSS to specify a height of 8 pixels.
The CSS source string has to be differentiated for Gtk pre and post
3.20, because Gtk 3.20 introduced some breaking changes in the way CSS
is handled.
References:
[1] Migrating from GTK+ 2.x to GTK+ 3 - Parsing of custom resources
https://developer.gnome.org/gtk3/stable/gtk-migrating-GtkStyleContext-parsing.html
[2] Gtk3 Reference Documentation - Changes in GTK+ 3.20
https://developer.gnome.org/gtk3/stable/ch32s10.html
[3] Gnome/HowDoI - Custom Style
https://wiki.gnome.org/HowDoI/CustomStyleCloses#7 - Port to Gtk3
Now that we are compiling against Gtkmm3 there are missing declarations
of Gtkmm identifiers due to changes in Gtkmm internal header structure.
All we have to do is bring back the declarations by including the
appropriate headers where needed.
Add necessary Gtkmm header includes.
Closes#7 - Port to Gtk3
GParted uses Gtk::Menu_Helpers::MenuList helper class to access
individual menu items. This helper class made it easy to navigate menu
items by index.
Gtk::Menu_Helpers::MenuList was removed in the switch from Gtkmm2 to
Gtkmm3 [1]. Instead, use a separate std::map<Gtk::MenuItem*> to keep
track of individual Gtk::MenuItem objects.
Reference:
[1] Gtkmm 3 commit "MenuShell: Remove items()." removed the code
c8e47b0db5Closes#7 - Port to Gtk3
Resizing a file system mounted read-only fails. Example:
# mkfs.btrfs /dev/sdb1
# mount -o ro /dev/sdb1 /mnt/1
In GParted try to resize partition sdb1. The operation fails like this:
Grow /dev/sdb1 from 512.00 MiB to 1.00 GiB (ERROR)
* calibrate /dev/sdb1 (SUCCESS)
* grow partition from 512.00 MiB to 1.00 GiB (SUCCESS)
* grow filesystem to fill the partition (ERROR)
* btrfs filesystem resize 1:max '/mnt/1' (ERROR)
Resize '/mnt/1' of '1:max'
ERROR: unable to resize '/mnt/1': Read-only file system
See GitLab issue for the testing results of attempting to online resize
all supporting file system while mounted read-only. No file system
allows online resizing while mounted read-only, except for reiserfs.
Issue #10 - Gparted fails to resize btrfs partition that is mounted
read-only
https://gitlab.gnome.org/GNOME/gparted/issues/10
Fix by preventing online resizing of *all* file systems mounted
read-only, including reiserfs. Instead of displaying the resize dialog
in this case, display an information dialog explaining why the partition
can't be resized. This is similar to what happens when attempting to
create a new partition on a disk without a partition table. The new
dialog looks like:
(!) Unable to resize read-only file system /dev/sdb1
The file system can not be resized while it is mounted read-only.
Either unmount the file system or remount it read-write.
[ OK ]
Closes#10 - Gparted fails to resize btrfs partition that is mounted
read-only
Usage of members label_device_info1 and label_device_info2 was removed
in this commit from 2004.
8ae5ebb2e6
several (mostly) i18n related fixes/cleanups
To keep password dialog open, just keep running it in a loop performing
LUKS mapping unlock attempts with the entered passphrase until it
succeeds or the dialog is cancelled or closed. This is the same model
that is already used for the File Support System dialog and how the
[Rescan For Supported Actions] button is implemented.
Also any error from attempting to open the LUKS mapping is no longer
displayed in a separate error dialog or at all. Will add some sort of
error reporting into the password entry dialog in a following commit.
Creates new method Win_GParted::open_encrypted_partition() which handles
the non UI parts of attempting to open an encrypted partition. Running
"cryptsetup luksOpen" and updating the stored passphrase as needed.
Bug 795617 - Implement opening and closing of LUKS mappings
Add new item to the partition menu to allow the user to open and close
the LUKS mapping. However for now the menu item is always disabled and
there is no implementation behind it to actually open or close the LUKS
mapping. Fragment of the partition menu is now:
...
Format to >
-----------------
Open Encryption <- New menu item
Mount
-----------------
Name Partition
...
Has to be two separate menu items to clearly represent to the user that
LUKS mappings and file system mounting are two separate busy states.
And also in the case of an open but unmounted file system to offer both
actions; close encryption and mount file system.
The text of the menu item automatically changes similarly to how it does
for the Mount/Unmount, Swapon/Swapoff, Activate/Deactivate item
depending on the state of the LUKS mapping. For open LUKS mappings it
will show "Close Encryption" and for all other cases (closed LUKS
mapping or partition is not encrypted) "Open Encryption". Again similar
to how the default of "Mount" is shown for unallocated and unknown
partitions.
Bug 795617 - Implement opening and closing of LUKS mappings
In preparation for adding the ability to toggle the encryption busy
state (open/close the encryption volume), rename existing members to
reflect that they are related to changing the file system state. (Swap
and LVM2 Physical Volumes are handled as file systems by GParted).
class Win_GParted renaming:
MENU_TOGGLE_BUSY -> MENU_TOGGLE_FS_BUSY
allow_toggle_busy_state() -> allow_toggle_fs_busy_state()
toggle_busy_state() -> toggle_fs_busy_state()
check_toggle_busy_allowed() -> check_toggle_fs_busy_allowed()
Bug 795617 - Implement opening and closing of LUKS mappings
There are too many different types of things named "filesystem" in the
GParted code with the potential to cause confusion. Namely:
std::vector<FS> FILESYSTEMS
Vector of file system capabilities.
class FileSystem Base class interfacing to file system
specific executables for querying and
modification.
enum FILESYSTEM Symbolic constants representing each file
system type.
Many recent written or re-written functions already used a variable
named fstype. Rename enum FILESYSTEM to enum FSType to clearly
distinguish it from the other things with very similar names. Only
changing the name of the enumeration, not the name of variables of that
type too because that is a lot more lines of code and those can be
changed when the relevant code is re-written.
Using the default MiB alignment, creating an MSDOS logical partition
between two other existing logical partitions fails with this error
dialog:
(-) <b>An error occurred while applying the operations</b>
See the details for more information.
<b>IMPORTANT</b>
If you want support, you need to provide the saved details!
See http://gparted.org/save-details.htm for more information.
[ OK ]
and these operation details:
+ libparted messages
- Unable to satisfy all constraints on the partition.
This bug was introduced by this commit included in GParted 0.23.0:
90e3ed68fc
Shallow copy Device object into Operation object (#750168)
The commit message claimed that the deep copied Partition objects inside
the Device inside the Operation object are never accessed. This turned
out not to be true. Win_GParted::Add_Operation() uses them as part of
snap_to_alignment() which updates requested partition boundaries to
account for alignment requirements and the space needed for EBR
(Extended Boot Record) preceding logical partitions.
In this case the new logical partition was trying to be created over the
top of the EBR for the following logical partition because
snap_to_alignment() wasn't aware of its existence.
Fix by making Add_Operation() and snap_to_alignment() refer to the
current device, as displayed in the UI, rather than the shallow copy
included in the Operation object. Hopefully now it is true that the
not copied vector of Partition objects in the Device object in each
Operation object are never accessed.
Bug 779339 - enforce at least 1 MiB "free space following"
It made the code look a little messy, is easily resolved in the build
system and made the dependencies more complicated than needed. Each
GParted header was tracked via multiple different names (different
numbers of "../include/" prefixes). For example just looking at how
DialogFeatures.o depends on Utils.h:
$ cd src
$ make DialogFeatures.o
$ egrep ' [^ ]*Utils.h' .deps/DialogFeatures.Po
../include/DialogFeatures.h ../include/../include/Utils.h \
../include/../include/../include/../include/../include/../include/Utils.h \
../include/../include/../include/Utils.h \
After removing "../include/" from the GParted header #includes, just
need to add "-I../include" to the compile command via the AM_CPPFLAGS in
src/Makefile.am. Now the dependencies on GParted header files are
tracked under a single name (with a single "../include/" prefix). Now
DialogFeatures.o only depends on a single name to Utils.h:
$ make DialogFeatures.o
$ egrep ' [^ ]*Utils.h' .deps/DialogFeatures.Po
../include/DialogFeatures.h ../include/Utils.h ../include/i18n.h \
These two methods had a lot of repeated and common code. Both determine
if the partition has any pending operations, notify the user that
changing the busy status can not be performed, and report any errors
when changing the status.
Extract the common code into sub-functions check_toggle_busy_allowed()
and show_toggle_failure_dialog() to handle showing the message dialogs.
Also refactor toggle_busy_state() to make it clear that it handles 5
cases of swapon, swapoff, activate VG, deactivate VG and unmount file
system.
Also remove out of date comment near the top of toggle_busy_state()
stating there can only be pending operations for inactive partitions is
out of date. Some file systems can be resized while online and
partition naming is allowed whatever the busy status of the file system.
Bug 775932 - Refactor mostly applying of operations
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 52a2a9b00a
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
If the GParted main window is closed before the initial device load
completed gpartedbin never exits. The main window closes but the
process sits there idle forever. Subsequently running GParted reports
this error:
# gparted
The process gpartedbin is already running.
Only one gpartedbin process is permitted.
If the user is running GParted from a desktop menu they will never see
this error so they will never know why GParted won't start any more.
More technically, it is if the main window is closed before the
Win_GParted::on_show() callback completes.
I assume the Gtk main loop doesn't setup the normal quit handling until
the on_show() callback finishes drawing the main window for the first
time. Following this hint [1], move the initial device load from the
on_show() callback to immediately after it completes by using a run once
idle callback setup in on_show().
This looks exactly the same to the user except now gpartedbin exits when
the main window is closed during the initial device load. Note that
GParted finished the device load before exiting. This is exactly the
same as happens when exiting during subsequent device refreshes.
[1] How to know when a Gtk Window is fully shown?
http://stackoverflow.com/questions/14663212/how-to-know-when-a-gtk-window-is-fully-shown
"If you want to know when your main window appears the first time,
it is far easier (and saner) add a g_idle_add after your show_all
call."
Bug 771816 - GParted never exits if the main window is closed before the
initial device load completes
Change Win_GParted::copied_partition from Partition object which is
copied by value into a pointer to a Partition object object which is
allocated, copy constructed and deleted. Required as part of the
polymorphic implementation of Partitions.
As before when managing the lifetime of pointers to objects in a class
the Big 3 of destructor, copy constructor and copy assignment operator
need to be considered. A destructor is added to finally delete
copied_partition. A single Win_GParted object is only ever created and
destroyed in main(). The class is never copy constructed or copy
assigned. Make the compiler enforce this with private declarations and
no implementations.
Bug 759726 - Implement Partition object polymorphism
Libsigc++2 version 2.5.2 and later removed header file
<sigc++/class_slot.h>. Quoting the NEWS file for version 2.5.2:
Remove useless headers: sigc++/class_slot.h ...
Libsigc++2 version 2.5.2 NEWS file:
https://git.gnome.org/browse/libsigc++2/tree/NEWS?id=2.5.2
Bug 756035 - GParted does not compile with newer gtkmm libraries in
Fedora 23
Since this commit earlier in the patchset the second optional parameter
of method Win_GParted::Add_Operation() is no longer used. Remove it.
Replace open coded merge of resize/move into create operation
(#755214)
Bug 755214 - Refactor operation merging
Creation of the various operations involved various implicit rules about
how the different types of operations were merged in different cases.
This was open coded in each ::activate_*() method. Abstract this into
new merge_operations() method and make the merging rules explicitly
specified.
NOTE:
The removal of operation type checking in the MERGE_LAST_WITH_ANY cases
is not a problem because all the Operation*::merge_operations() methods
ensure the operation types match as part of the merge attempt.
Bug 755214 - Refactor operation merging
Rename Win_GParted::Merge_Operations() to merge_two_operations(). To
reflect what it does and in preparation for further refactoring of the
code.
Be more strict on the validation of the first and second indexes. The
first operation must also be before the second operation in the
operation[] vector. (It is actually a programming bug if first and
second fail validation. However so far g_assert() is only being used to
validate pointers, which if wrong would likely cause the program to
eventually crash when dereferenced later. In this case a bug would
merely cause the incorrectly specified pair of operations to not be
merged).
Move validate_display_partition_ptr() declaration in the header file to
be in the same ordering as it's definition in the source file.
Bug 755214 - Refactor operation merging
As previous commit, display_partitions is now a Win_GParted member
variable so checking for the existence of an extended partition can be
localised where it is used.
Remove index_extended member variable and localise the same checking in
activate_new().
Now that display_partitions is a Win_GParted member variable and
therefore available throughout the class, since commit [1], calculation
of primary_count can be localised in max_amount_prim_reached() where it
is used.
Implements a FIXME and removes primary_count as a member variable.
[1] 545b75d957
Move vector of partition objects to a Win_GParted class member (#750168)
Further ensure that a bug doesn't get introduced with the use of
selected_partition_ptr, by asserting that it points to a current
partition object in the vector of display partitions.
After deliberately breaking the code so that selected_partition_ptr
points to some other partition object, trying to display the Information
dialog causes this crash:
======================
libparted : 2.4
======================
**
ERROR:Win_GParted.cc:989:void GParted::Win_GParted::set_valid_operations(): assertion failed: (valid_display_partition_ptr( selected_partition_ptr ))
Aborted (core dumped)
At this point in the code:
973 void Win_GParted::set_valid_operations()
974 {
...
986 // No partition selected ...
987 if ( ! selected_partition_ptr )
988 return ;
>> 989 g_assert( valid_display_partition_ptr( selected_partition_ptr ) ); // Bug: Not pointing at a valid display partition object
Bug 750168 - Reduce the amount of copying of partition objects
Now that TreeView_Details and DrawingAreaVisualDisk classes store and
pass pointers to partition objects in the Gtk signal callbacks, change
the selected partition into a pointer too.
Bug 750168 - Reduce the amount of copying of partition objects
Change from passing a reference to the selected partition, to passing a
pointer to the selected partition in the signal_partition_selected
callbacks between the disk graphic, partition list and core GUI modules.
This is an enabler for the following patches.
Bug 750168 - Reduce the amount of copying of partition objects
Win_GParted::Refresh_Visual() used a local variable containing a copy of
the vector of partitions in the current device to be displayed. After
visually applying pending operations it loaded copies of each partition
object into the GUI widgets to display the disk graphic and partition
list, DrawingAreaVisualDisk and TreeView_Details classes respectively.
When a partition is selected in the UI, again a partition object is
copied. Also several of the partition dialogs, including the
information dialog, take a copy of the partition object. All these are
copies of the same set of partition objects, those currently being
displayed in the UI.
Move the vector of displayed partitions from a local variable in
Refresh_Visual() to a Win_GParted member variable. This will allow for
the above cases to be changed to used pointers and references to the
same set of partition objects.
The valid lifetime of pointers to elements in this partition object
vector is from one refresh to the next, when the vector is cleared and
repopulated with a new set of partition objects. This is exactly what
is needed as the GUI widgets are reloaded on each refresh, the selected
partition is reset and none of the partition dialog objects exist.
Dialog objects being created and destroyed on each use.
On the other hand some copies of partition objects currently being
displayed, still need to be made because they have lifetimes which need
to last longer than the next call to Refresh_Visual(). Specifically the
source of the copy partition and the partition objects copied into the
in the list of pending operations.
Bug 750168 - Reduce the amount of copying of partition objects
Embedded devices (Android) use GPT partition names to identify
partitions, instead of file system labels. Add support for viewing and
changing them.
As partition names are used to provide unique identification they are
never copied when copying the contents of one partition to another.
Note that GNU/Linux uses file system labels, UUIDs or device names for
identification during the boot process and afterwards so while partition
names can be used, they are optional and purely for user information.
Bug 741424 - Add support for GPT partition names
The device combobox was getting drawn blank, then getting drawn again
with the selected device. This was happening because at the start of
Win_GParted::refresh_combo_devices() the GTK model behind the combobox,
liststore_devices, was cleared, changing the active item, causing the
combobox to get redrawn empty. After the GTK model had been repopulated
the active item was reset causing the comboxbox to get redrawn again,
now showing the selected device. Call flow:
Win_GParted::refresh_combo_devices()
liststore_devices->clear()
//Gtk::Combobox emits signal_change. Registered callbacks
//called.
Win_GParted::combo_devices_changed()
Win_GParted::Refresh_Visual()
...
...
combo_devices.set_active(current_device);
//Gtk::Combobox emits signal_change. Registered callbacks
//called.
Win_GParted::combo_devices_changed()
Win_GParted::Refresh_Visual()
...
This has always been the case, since the device combobox was first added
to GParted before version 0.1 by commit:
3a4b43e0ad
replaced deprecated OptionMenu with ComboBox ...
Fix by temporarily blocking the devices comboxbox from emitting
signal_changed while the GTK model behind the combobox is recreated.
However, since automatic selection of the largest free space was added
[1] in GParted 0.15.0, a more noticeable flashing redraw issue was
caused in which the partition graphic and partition list were both drawn
blank then redrawn fully populated. Some distributions were not
affected by this at all, some only experienced a single flash and others
suffered from two or more flashing redraws. Some affected
distributions: CentOS 5.10, 6.5, 7.0, Debian 6, Fedora 14, 19, 20,
Ubuntu 13.10, Xubuntu 14.04 LTS. Did not occur on Kubuntu 12.04 LTS.
[1] 5b53c12f6e
Select largest unallocated partition by default (#667365)
Bug #696149 - Double refresh of display introduced with default
unallocated space
This is part of parent bug:
Bug #721455 - Obsolete info in license text on multiple modules
and GNOME Goal:
https://wiki.gnome.org/Initiatives/GnomeGoals/Proposals
* verify all source files to make sure they have a license and a
copyright, and that both are up-to-date
Bug #721565 - License text contains obsolete FSF postal address
Include guards need to be unique within GParted code and all included
library header files.
http://en.wikipedia.org/wiki/Include_guard#Difficulties
Use this model for all include guards:
#ifndef GPARTED_FILE_NAME_H
#define GPARTED_FILE_NAME_H
...
#endif /* GPARTED_FILE_NAME_H */
Closes Bug #539297 - Make include guards unique
It was possible to make GParted crash by adding a label, check or new
UUID operation and then applying the operation before the view of
pending operations had finished fully opening. The operation would be
successfully applied but GParted would crash afterwards.
The fault was that Add_Operation() still enabled the Undo and Apply
buttons and processed the GTK event loop before merging the list of
pending operations. Faulty code flow went like this:
activate_*()
Add_Operation()
Add operation to the operations[] vector
Enable Undo and Apply buttons
Refresh_Visual()
Process GTK event loop
Process Apply button callback applying operations,
refreshing display and clearing operations[] vector
Merge operations in the operations[] vector
<< Core dump here >>
Merge_Operations()
Refresh_Visual()
This faulty code flow came about when merging of operations was added
and it didn't appreciate that the operations[] vector could have been
processed and cleared by Add_Operations() before the merge step.
Relevant commit:
b10349ae37
Merge overlapping operations (#438573)
Fragment of code in the label operation case:
2454 void Win_GParted::activate_label_partition()
2455 {
...
2472 Add_Operation( operation ) ;
2473
2474 // Verify if the two operations can be merged
2475 for ( unsigned int t = 0 ; t < operations .size() - 1 ; t++ )
2476 {
2477 if ( operations[ t ] ->type == OPERATION_LABEL_PARTITION )
2478 {
2479 if ( Merge_Operations( t, operations .size() - 1 ) )
2480 break;
2481 }
2482 }
Commentary in the crashing label operation case:
2472 The pending operation was already applied when Add_Operation()
returned resulting in the operations[] vector being cleared
setting its size to 0.
2475 The return type of operations.size() is an unsigned integral, so
the upper limit of the for loop is t < 0UL - 1. Assuming a
32-bit machine that's t < 4294967295.
2477 operations[] vector is access from out of bounds offset 0
upwards until unallocated memory is accessed resulting in a core
dump.
Fix this by not enabling the Undo and Apply buttons and processing the
GTK event loop until after merging of operations has been performed.
Fixed code flow goes like this:
activate_*()
Add_Operation()
Add operation to the operations[] vector
Merge operations in the operations[] vector
Merge_Operations()
show_operationslist()
Enable Undo and Apply buttons
Refresh_Visual()
Process GTK event loop
Process Apply button callback applying operations,
refreshing display and clearing operations[] vector
Not allowing the operations list to be process until after the merge
step is the be correct ordering. This also prevents the new operation
from flashing up in the operations list and then immediately
disappearing if merged. In the case of adding the first operation,
delaying enabling the Undo and Apply buttons is enough as the buttons
were previously disabled preventing the operation being applied before
the merge. In the case of adding further operations, processing of the
GTK event loop must also be delayed until after the merge to prevent the
operations being applied before the merge. Although that window of
opportunity would only be microseconds.
Bug #699452 - Crash when applying operations before pending operations
fully displayed
Move some code into new create_format_menu_add_item() sub-function which
adds one file system entry to the Partition --> Format to -->
(file system list) menu.
Bug #688882 - Improve clearing of file system signatures
Win_Gparted and Dialog_Progress were creating threads to perform most
functions in the background. Most of the time, the only reason the
threads blocked was to execute an external command. The external command
execution has been changed to spawn the command asynchronously and wait
for completion with a nested main loop. While waiting for completion,
the pipe output is captured via events. In the future, this will allow
for it to be parsed in real time to obtain progress information.
Those tasks in GParted_Core that still block now spawn a background thread
and wait for it to complete with a nested main loop to avoid hanging the
gui.
Part of Bug #685740 - Refactor to use asynchronous command execution
Win_Gparted and Dialog_Progress were looping on Gtk::Main::events_pending()
and iteration() with usleeps in between. Use a full mainloop instead and
a proper timeout to trigger pulsebar updates instead of usleeps.
Part of Bug 685740 - Refactor to use asynchronous command execution
Mike Fleetwood