When multiple devices are named on the command line and (after sorting
and removing duplicates) the device following a non-existent or invalid
one is not checked for usability [1]. In most situations this isn't
noticed as the device gets skipped at the "Searching ... partitions"
step instead. However as seen in bug 755495 and commit [2]
checking usability matters.
For example (on CentOS 6.5) a large sector disk device can be edited
when it follows a non-existent or invalid device named on the command
line:
# modprobe scsi_debug dev_size_mb=128 sector_size=4096
# fgrep scsi_debug /sys/block/*/device/model
/sys/block/sdd/device/model:scsi_debug
# ./gpartedbin /dev/does-not-exist /dev/sdd
======================
libparted : 2.1
======================
Could not stat device /dev/does-not-exist - No such file or directory.
Device /dev/sdd has a logical sector size of 4096. Not all parts of GNU Parted support this at the moment, and the working code is HIGHLY EXPERIMENTAL.
/dev/sdd: unrecognised disk label
When erasing a device don't skip confirming the following device is
usable.
[1] Usable device as implemented by useable_device()
Must not have a large sector size when GParted is built with an old
version of libparted which doesn't support large sector sizes and
must be able to read the first sector.
[2] 362b2db331
Check disks named on the command line are safe to use too (#755495)
Bug 756434 - GParted dumps core when passing non-existent or invalid
device on the command line
A non-existent or invalid disk device named on the command line caused
two libparted dialogs to be displayed repeatedly on every refresh. This
was because the device was only removed from the 'device_paths' vector
when it wasn't usable [1]; not when it didn't exist or was invalid, when
the libparted ped_device_get() call failed. Fix this.
[1] Usable device as implemented by useable_device()
Must not have a large sector size when GParted is built with an old
version of libparted which doesn't support large sector sizes and
must be able to read the first sector.
Bug 756434 - GParted dumps core when passing non-existent or invalid
device on the command line
Naming a non-existent or invalid disk device on the command line causes
GParted to dump core. Non-existent device looks like this:
# ./gpartedbin /dev/does-not-exist
======================
libparted : 2.4
======================
Could not stat device /dev/does-not-exist - No such file or directory.
Could not stat device /dev/does-not-exist - No such file or directory.
Backtrace has 10 calls on stack:
10: /lib64/libparted.so.0(ped_assert+0x31) [0x7fcfd10b3e61]
9: /lib64/libparted.so.0(+0x3fdfc12a0c) [0x7fcfd10b4a0c]
8: /home/mike/bin/gpartedbin-0.23.0-master-63-g23b5ba4() [0x455028]
7: /home/mike/bin/gpartedbin-0.23.0-master-63-g23b5ba4() [0x455090]
6: /home/mike/bin/gpartedbin-0.23.0-master-63-g23b5ba4() [0x4550d5]
5: /home/mike/bin/gpartedbin-0.23.0-master-63-g23b5ba4() [0x46723f]
4: /usr/lib64/libglibmm-2.4.so.1() [0x3ff5834a8d]
3: /lib64/libglib-2.0.so.0() [0x3fe086a374]
2: /lib64/libpthread.so.0() [0x3fdf407a51]
1: /lib64/libc.so.6(clone+0x6d) [0x3fdf0e893d]
Assertion (dev != NULL) at device.c:227 in function ped_device_open() failed.
Aborted (core dumped)
And with an invalid device the output looks like this:
# ./gpartedbin /dev/zero
======================
libparted : 2.4
======================
The device /dev/zero is so small that it cannot possibly store a file system or partition table. Perhaps you selected the wrong device?
Error fsyncing/closing /dev/zero: Invalid argument
The device /dev/zero is so small that it cannot possibly store a file system or partition table. Perhaps you selected the wrong device?
Error fsyncing/closing /dev/zero: Invalid argument
Backtrace has 10 calls on stack:
...
[Same as above]
Bisected the cause to this commit from 2015-03-09 in GParted 0.22.0. It
claimed to make no functional change. That turned out not to be true.
51ac4d5648
Split get_device_and_disk() into two (#743181)
Fix by simply adding the missed if condition in get_device().
Bug 756434 - GParted dumps core when passing non-existent or invalid
device on the command line
For probed DMRaid devices (when not using libparted DMRaid support)
GParted waits up to 1 second for udev to have processed all events and
created the /dev entries after starting each DMRaid array. This was
added by this commit from 2009-09-02:
e7352a5000
Ensure /dev file system device entries created before adding device
Do the same for devices named on the command line too.
Order named disk devices so that they appear in the combo box in the
same order which they would when probed. Also remove duplicates so that
the same disk devices aren't scanned multiple times and appear
duplicated in the UI.
Try this; it used to take ages to load and looked weird:
# gparted /dev/sda /dev/sdb /dev/sda /dev/sdb /dev/sda /dev/sdb
Bug 755495 - GParted allowing partitioning of large sector devices
specified on the command line, when built with old
libparted which doesn't support it
When probing for disk devices GParted ensures that libparted is capable
of handling the sector size safely and that it is a real disk before it
is shown in the UI. However when disk devices are named on the command
line none of these checks are performed.
Libparted versions before v2.2 can only safely handle a sector size of
512 bytes. Therefore on old distributions with libparted < v2.2 GParted
allows unsafe editing of disk devices with larger sector sizes when they
are named on the command line. Known to affect these distributions:
RedHat/CentOS 5 (parted 1.8.1)
RedHat/CentOS 6 (parted 2.1)
For example (on CentOS 6.5) large sector disk device is ignored when
probing:
# modprobe scsi_debug dev_size_mb=128 sector_size=4096
# fgrep scsi_debug /sys/block/*/device/model
/sys/block/sdd/device/model:scsi_debug
# gparted
======================
libparted : 2.1
======================
Device /dev/sdd has a logical sector size of 4096. Not all parts of
GNU Parted support this at the moment, and the working code is
HIGHLY EXPERIMENTAL.
Ignoring device /dev/sdd with logical sector size of 4096 bytes.
GParted requires libparted version 2.2 or higher to support devices
with sector sizes larger than 512 bytes.
However when the device is named it is not ignored and can be edited:
# gparted /dev/sdd
======================
libparted : 2.1
======================
Device /dev/sdd has a logical sector size of 4096. Not all parts of
GNU Parted support this at the moment, and the working code is
HIGHLY EXPERIMENTAL.
/dev/sdd: unrecognised disk label
Apply the same validity checks to disk devices named on the command line
as to probed ones.
Bug 755495 - GParted allowing partitioning of large sector devices
specified on the command line, when built with old
libparted which doesn't support it
Abstract code checking sector size and ensuring the first sector of a
candidate disk device can be read into new
GParted_Core::useable_device() method.
Bug 755495 - GParted allowing partitioning of large sector devices
specified on the command line, when built with old
libparted which doesn't support it
I missed another case of 'index' may be used uninitialised warning in
OperationDelete::apply_to_visual(). Indent a code block within an if
clause so that the compiler can confirm that the 'index' local variable
isn't used uninitialised. Prevent this compiler warning:
OperationDelete.cc: In member function 'virtual void GParted::OperationDelete::apply_to_visual(std::vector<GParted::Partition, std::allocator<GParted::Partition> >&)':
OperationDelete.cc:34: warning: 'index' may be used uninitialized in this function
Bug 755214 - Refactor operation merging
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
Glibmm 2.45.40 and later uses features in the ISO C++ 2011 standard.
The NEWS file [1] says:
Changes in 2.46 compared to 2.44:
General:
* Use, and require C++11, using features such as move operations,
noexcept auto, = delete, nulltpr, override.
(Murray Cumming, Kjell Ahlstedt)
Also found this by Murray Cumming [2]:
glibmm 2.45.40 requires C++11, both for its own build and by any
apps that use it. gtkmm also now requires C++11.
I think you are seeing a symptom of building the application without
C++11 support. For instance, using CXXFLAGS="--std=c++11", though
you'd be better of using an m4 macro such as
AX_CXX_COMPILE_STDCXX_11().
Without enabling C++11 compiler features, compilation of GParted with
the latest glibmm library fails with errors such as these:
/usr/include/glibmm-2.4/glibmm/ustring.h:267:14: error: expected ';' at end of member declaration
~ustring() noexcept;
^
/usr/include/glibmm-2.4/glibmm/ustring.h:267:14: error: 'noexcept' does not name a type
~ustring() noexcept;
^
/usr/include/glibmm-2.4/glibmm/ustring.h:267:14: note: C++11 'noexcept' only available with -std=c++11 or -std=gnu++11
[1] glibmm NEWS file
https://git.gnome.org/browse/glibmm/tree/NEWS?id=934e8290ce913b12e251ea617d0fc8ac53c385c6
[2] https://bugs.launchpad.net/ubuntu/+source/glibmm2.4/+bug/1478367
Bug 756035 - GParted does not compile with newer gtkmm libraries in
Fedora 23
Older distributions don't include Autoconf Archive at all and newer
distributions don't always include it. Therefore we have to have a
local copy of the AX_CXX_COMPILE_STDCXX_11 macro with GParted.
Bug 756035 - GParted does not compile with newer gtkmm libraries in
Fedora 23
Add recognition for work done by Albert Young to prevent a hang in
GParted labeling FAT16/32 if label contained illegal characters.
Bug 755608 - Labeling fat16/fat32 partitions hangs if certain
characters included in label
GParted waits forever when attempting to set a FAT16/32 file system
label which contains prohibited characters [1][2]. This is because
mlabel asks a question and is waiting for input. Force cancelling the
operation doesn't work either as GParted sends signal 2 (interrupt i.e.
[Ctrl-C]) but mtools commands specifically ignores this and a number of
other signals. Have to kill mlabel with signal 9 (kill) to regain
control of GParted.
Mlabel command with prohibited characters in the label:
# export MTOOLS_SKIP_CHECK=1
# mlabel ::"MYLABEL/ " -i /dev/sdb10
Long file name "MYLABEL/ " contains illegal character(s).
a)utorename A)utorename-all r)ename R)ename-all
s)kip S)kip-all q)uit (aArRsSq):
Remove prohibited characters from FAT16/32 file systems labels when
creating and labelling them. Also upper case the label to meet label
requirements [1][2]. This silently corrects the label and the actual
label applied will be displayed when GParted refreshes after applying
the operation.
[1] Microsoft TechNet: Label
https://technet.microsoft.com/en-us/library/bb490925.aspx
[2] Replicated in Wikikedia: label (command)
https://en.wikipedia.org/wiki/Label_%28command%29
Bug 755608 - Labeling fat16/fat32 partitions hangs if certain characters
included in label
These member variables store no Operation class information and were
being used as local variables. Replace with local variables.
Also indent a code block within an if clause so that the compiler can
confirm that the new local variable isn't used uninitialised. Prevents
this compiler warning:
OperationResizeMove.cc: In member function 'void GParted::OperationResizeMove::apply_normal_to_visual(std::vector<GParted::Partition, std::allocator<GParted::Partition> >&)':
OperationResizeMove.cc:125: warning: 'index' may be used uninitialized in this function
Bug 755214 - Refactor operation merging
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
This is the equivalent case fixed in the earlier commit, but now using
copy/paste to create the second new partition rather than plain new.
Fix visually re-apply create operation in create-create-grow-first
sequence (#755214)
Start with an existing partition as a copy source. Then this sequence
of operations will cause the copy partition to disappear from the disk
graphic:
1) create new #1,
2) copy existing / paste into unallocated leaving space preceding,
3) resize new #1 larger.
There are two different types of copy operation. The first is copy into
unallocated space creating a new partition which needs treating the same
as create new operation. The second is copy into existing partition
which needs treating the same as the other operations which don't change
the boundaries of the partition. Fix apply_to_visual() accordingly.
Bug 755214 - Refactor operation merging
Move the code from OperationCreate::apply_to_visual() into new method
Operation::insert_new() in the parent class. This is in preparation for
the following commit.
Bug 755214 - Refactor operation merging
The apply_to_visual() method for the change UUID, format, label file
system and name partition operations duplicated identical code. This
code was just substituting the partition in the disk graphic vector with
the new partition recorded in the operation, as none of these operations
change the partition boundaries. Move this duplicate code into the
parent class in new method Operation::substitute_new().
Bug 755214 - Refactor operation merging
After previous commit "Replace open coded merge of resize/move into
create operation (#755214)" the second created partition would disappear
from the disk graphic in the following sequence: create new #1, create
new #2 leaving space preceding, resize #1 larger. The create new #2
operation still existed and was shown in the operation list. It was
just that it disappeared from the disk graphic.
Remember that when each operation is created it records the partition,
or the unallocated space, to which the operation is applied at the time
the operation is created in the partition_original member variable. In
the above sequence the resize #1 larger operation was merged back into
the create new #1 operation. When visually re-applying the create
new #1 operation to the disk graphic, it left a smaller unallocated
partition following it. This was smaller than the unallocated partition
recorded in the create new #2 operation, hence it failed to visually
re-apply to the disk graphic.
The insight to fix this is that it doesn't matter what size the
unallocated space was when the create new operation was constructed. It
only matters that the new partition to be created fits in the available
unallocated space currently in the disk graphic.
Bug 755214 - Refactor operation merging
This information is already documented in the existing comments
associated with the calls to merge_operations() and assignments to the
mergetype variables. The table just summaries the rules together in one
place.
Bug 755214 - Refactor operation merging
For the case of resizing/moving a new, not yet created partition,
activate_resize() open coded the merge operation. Again this code has
existed in GParted since before version 0.0.5 and the current code
history in Git.
Replace the necessary code so that an explicit merge_operations() call
is used instead; along with the other case of resizing/moving an
existing partition.
NOTES:
This commit changes the merge direction. The old coded merged forward
by removing the old create operation and adding a new create operation
with the new size. This was bad because with multiple pending create
operations, each merged resize operation reordered those create
operations. Then when the operations were applied the partitions were
created and therefore numbered in a different order to that shown in
disk graphic.
The new code merges backwards by updating the initial create operation
with the new size. This maintains the create operation order so that
when applied the partitions are numbered in the same order as shown in
the disk graphic.
Bug 755214 - Refactor operation merging
For the case of formatting a new, not yet created partition,
activate_format() open coded the merge operation. This code has existed
in GParted since before version 0.0.5 and the current code history in
Git.
Replace the necessary code so that an explicit merge_operations() call
is used instead; along with the other case of formatting an existing
partition.
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
Win_GParted::Merge_Operations() method was modifying the internals of
Operation* objects; in particular the partition_new member variable.
This is breaking data hiding and encapsulation tenant of object oriented
programming.
Implement exactly the same operation merge semantics, but hide the
manipulation of the internals of the Operation* objects within the
Operation* classes themselves.
Bug 755214 - Refactor operation merging
... before refactoring the code.
See the commit message from 2011-10-05 for details of what operations,
available at that time, on the same partition can be merged and in what
cases:
b10349ae37
Merge overlapping operations (#438573)
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)
Perform a copy, reformat source and paste sequence in GParted. When the
source is a primary partition everything works as expected, with the
newly pasted partition reflecting the reformatted source. However when
the source is a logical partition GParted thinks it is pasting the
original source, rather than the reformatted source. The same is also
true for other file system manipulation operations: resize, file system
label and new UUID. It is just that reformatting the source to a
different file system type is the most obvious in the UI and causes the
most significantly wrong actions to be performed.
For example start with an ext4 logical partition, select it for copy,
format it to xfs and paste into a new partition. GParted thinks the
second operation will create a copy of an ext4 file system instead of
the xfs file system. When applied the operation details are:
Format /dev/sdd5 as xfs
+ calibrate /dev/sdd5
+ clear old file system signatures in /dev/sdd5
+ set partition type on /dev/sdd5
+ create new xfs file system
+ mkfs.xfs -f -L "" /dev/sdd5
Copy /dev/sdd5 to /dev/sdd (start at 131.00 MiB)
+ calibrate /dev/sdd5
+ check file system on /dev/sdd5 for errors and (if possible) fix them
+ e2fsck -f -y -v -C 0 /dev/sdd5
e2fsck: Subperblock invalid, trying backup blocks...
Resize inode not valid. Recreate? yes
...
/dev/sdd5: ***** FILE SYSTEM WAS MODIFIED *****
+ create empty partition
+ set partition type on /dev/sdd6
new partition type: ext4
+ copy file system of /dev/sdd5 to /dev/sdd6
using internal algorithm
...
GParted formatted sdd5 to xfs, but then the copy step ran e2fsck and
managed to resurrect the ext4 file system and then performed a block
copy of it to partition sdd6. The copy step should have ran xfs_repair
and used xfsdump | xfsrestore to copy the xfs file system. Afterwards
sdd5 contains both xfs and ext4 signatures.
# wipefs /dev/sdd5
offset type
----------------------------------------------------------------
0x438 ext4 [filesystem]
UUID: f0ed4247-76db-4d93-b3bc-c7da4a70f95e
0x0 xfs [filesystem]
UUID: 1ac8e7c3-0311-4c64-8e4a-b715a23ea0bd
This has been broken at least as far back as GParted 0.1.0.
Fix by simply refreshing the copy source partition object when it is a
logical partition too, as well as when it is a primary partition.
Bug 754827 - Copy, reformat source and paste a logical partition thinks
it's pasting the original file system
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
Replace open coding of the creation of the operation details for the
mlabel command used to set the label and UUID with calls to
FileSystem::execute_command() which will do it all. This also results
in the commands getting a time and check mark displayed in the operation
details.
Bug 754684 - Updates to FileSystem:: and Utils::execute_command()
functions
There has been an undocumented rule that external commands displayed in
the operation details, as part of file system manipulations, only get a
time and check mark displayed when multiple commands are needed, and not
otherwise. (GParted checks whether all commands are successful or not
regardless of whether a check mark is displayed in the operation details
or not).
EXCEPTION 1: btrfs resize
Since the following commit [1] from 2013-02-22, GParted stopped
displaying the timing for the btrfs resize command in the operation
details. It being part of a multi-command sequence to perform the step.
This is because FileSystem::execute_command() since the commit can only
check the exit status for zero / non-zero while timing and checking the
command status but btrfs resize needs to consider some non-zero statuses
as successful.
[1] 52a2a9b00a
Reduce threading (#685740)
EXCEPTION 2: ext2/3/4 move and copy using e2image
When use of e2image was added [2] the single command steps were timed
and check.
[2] 86111fe12a
Use e2image to move/copy ext[234] file systems (#721516)
EXCEPTION 3: fat16/32 write label and UUID
Uses Utils::execute_command() rather than FileSystem::execute_command()
so can be separately changed. See the following commit for resolution
of the final commands not yet timed and check mark displayed.
CHANGE:
Lets make a simpler rule of always displaying the time and a check mark
for all external commands displayed in the operation details. However
this makes several of the other single command actions need special exit
status handling because zero success, non-zero failure is not correct
for every case. Specifically affects resizing of reiserfs and check
repair of ext2/3/4, fat16/32, jfs and reiserfs.
After this change all external commands run as file system actions must
follow one of these two patterns of using the EXEC_CHECK_STATUS flag or
separately calling FileSystem::set_status() to register success or
failure of the command:
exit_status = execute_command(cmd, od, EXEC_CHECK_STATUS...);
or:
exit_status = execute_command(cmd, od, ...);
bool success = (exit_status == 0 || exit_status == OTHER_SUCCESS_VALUE...);
set_status(od, success );
Bug 754684 - Updates to FileSystem:: and Utils::execute_command()
functions
Change the two optional boolean parameters into a single optional flags
parameter which uses symbolically defined names. Makes reading the
execute_command() calls much easier to understand. (Implemented as bit
field using the same technique as used for Glib::SpawnFlags [1]).
This changes the calls thus:
execute_command(cmd, od) -> (cmd, od)
execute_command(cmd, od, false) -> (cmd, od, EXEC_NONE) // [2]
execute_command(cmd, od, true ) -> (cmd, od, EXEC_CHECK_STATUS)
execute_command(cmd, od, false, true) -> (cmd, od, EXEC_CANCEL_SAFE)
execute_command(cmd, od, true , true) ->
(cmd, od, EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE)
[1] SpawnFlags bitwise operators in
/usr/include/glibmm-2.4/glibmm/spawn.h.
[2] False and EXEC_NONE are the default values for the optional third
parameter before and after this change respectively and both mean
the same. This is being used in btrfs::resize() and being kept for
now despite it being the default.
Bug 754684 - Updates to FileSystem:: and Utils::execute_command()
functions
Use of execute_command_timed() was removed by this commit from
2013-02-22:
52a2a9b00a
Reduce threading (#685740)
Bug 754684 - Updates to FileSystem:: and Utils::execute_command()
functions
Curtis Gedak