All the dialogs which compose new or modified partition boundaries
create those partitions within the boundaries of the device. Therefore
trimming the partition boundaries to device boundaries never happens.
So replace this trimming with bug error reports.
Also add bug prefixes to the other error messages in
GParted_Core::valid_partition() too.
Closes#48 - Error when moving locked LUKS-encrypted partition
The previous commit moved the code from GParted_Core to
Dialog_Base_Partition class without making a single formatting change to
ensure the code was guaranteed to work the same within that larger
commit. Now reformat the code to current layout standards. Making it a
separate commit simplifies the effort for both changes and improves
bisectability.
Additionally to be sure there were no code changes,
Dialog_Base_Partition.cc was compiled to assembler code with and without
this change applied and the resultant assembler code compared. There
were no differences in the generated assembler code.
Start with the make generated g++ command for compiling
Dialog_Base_Partition.o; (1) remove the '-g' flag as inserted debugging
directives do differ; and (2) replace '-c -o Dialog_Base_Partition.o'
with '-S -o Dialog_Base_Partition.s' to produce assembler output instead
of object code.
Closes#48 - Error when moving locked LUKS-encrypted partition
Move snap_to_*() method calls from the point when all operations are
added to the list, to earlier when Resize/Move, Paste (into new) and
Create New dialogs are composing the new partition objects. In
particular for the Resize/Move operation, to just before updating the
file system usage.
This change finally resolves this bug.
Because of the dialog call chains into Dialog_Base_Partition,
snap_to_alignment() must be added into:
* Dialog_Base_Partition::prepare_new_partition() for the Resize/Move and
Paste (into new) dialogs; and
* Dialog_Partition_New::Get_New_Partition() for the Create New dialog.
Closes#48 - Error when moving locked LUKS-encrypted partition
The current device has to be passed to the dialog constructors and then
on to the Dialog_Base_Constructor. Note that the Dialog_Partition_New
constructor is already passed the current device, however it still needs
to pass it on to Dialog_Base_Constructor.
This is ready so that snap_to_*() methods can access the current device
when they are called from within these dialogs.
* Pass Parameter to Base Class Constructor while creating Derived class
Object
https://stackoverflow.com/questions/16585856/pass-parameter-to-base-class-constructor-while-creating-derived-class-objectCloses#48 - Error when moving locked LUKS-encrypted partition
Move setting of the new_partition object file system usage to after
everything else, specifically after free_space_before and strict_start.
This is because snap_to_*() use free_space_before and strict_start and
snap_to_alignment() is going to be called before the file system usage
is updated to avoid the error in this bug report.
Closes#48 - Error when moving locked LUKS-encrypted partition
None of the snap_to_*() methods report any errors so remove the unused
error parameter and return value.
Closes#48 - Error when moving locked LUKS-encrypted partition
PATCHSET OVERVIEW
A user had 2 adjacent partitions which were aligned to multiples of
33553920 bytes (32 MiB - 512 bytes), not MiB or cylinders. As far as
GParted is concerned this is not aligned. The second partition
contained closed LUKS encrypted data. Recreate this setup with:
# truncate -s 200G /tmp/disk.img
# losetup -f --show /tmp/disk.img
/dev/loop0
# sfdisk -u S /dev/loop0 << EOF
65535 2162655 83
2228190 78904140 83
EOF
# partprobe /dev/loop0
# echo -n badpassword | cryptsetup luksFormat /dev/loop0p2 -
When trying to move the second LUKS encrypted partition to the right by
any amount, with the default MiB alignment, GParted displays this error
dialog and fails to even queue the operation:
Could not add this operation to the list
A partition with used sectors (78907392) greater than its
length (78905344) is not valid
[ OK ]
Overview of the steps involved:
1. The Resize/Move dialog composed a new partition to start a whole
multiple of MiB after the end of the previous non-aligned partition.
The new partition also had it's size increased to a whole multiple of
MiB, to 78907392 sectors (38529 MiB) which was 1.59 MiB larger than
before. Neither the start or end of the new partition are aligned at
this point.
2. Win_GParted::activate_resize() applied the change back to the closed
LUKS partition object, additionally making the used sectors equal to
the partition size.
(To match the fact that when opened the LUKS mapping it will
automatically fill the new larger partition size).
3. GParted_Core::snap_to_mebibyte() then aligned the partition start and
end to whole MiB boundaries, reducing the partition size in the
process to 78905344 (38528 MiB).
4. GParted_Core::snap_to_alignment() reported the error saying that it
couldn't add the operation to the list because it was invalid to have
the file system used sectors larger than the partition size.
Fix this by having the snap to alignment adjustments applied before the
dialogs update any associated file system usage. Specifically the
Resize/Move, Paste (into new) and Create New dialogs as these are the
only ones which either create or modify partition boundaries.
Validation done by snap_to_alignment() will continue to occur at the
current point when the operation is added to the list.
THIS COMMIT
snap_to_alignment() is doing two different jobs, it is (1) optionally
adjusting the new partition boundaries for MiB or Cylinder alignment;
and (2) checking that the partition boundaries and file system usage are
valid.
Split those into two different functions (1) snap_to_alignment() and
(2) valid_partition(). For now valid_partition() still calls
snap_to_alignment() so there is no functional change with this commit.
Closes#48 - Error when moving locked LUKS-encrypted partition
We have to copy the dentry->d_name before calling closedir(). If not,
the string points to nothing and the test fails (It does not fail all
the time, but only by chance).
Confirmed using valgrind. Selected output from running the unit test
under valgrind:
$ valgrind --track-origins=yes ./test_blockSpecial
==25110== Memcheck, a memory error detector
...
==25110== Command: ./test_BlockSpecial
==25110==
Running main() from src/gtest_main.cc
[==========] Running 26 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 26 tests from BlockSpecialTest
...
[ RUN ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches
==25110== Invalid read of size 1
==25110== at 0x4C2C9B2: strlen (vg_replace_strmem.c:458)
==25110== by 0x40E7C4: length (char_traits.h:259)
==25110== by 0x40E7C4: append (basic_string.h:1009)
==25110== by 0x40E7C4: operator+<char, std::char_traits<char>, std::allocator<char> > (basic_string.h:2468)
==25110== by 0x40E7C4: get_link_name (test_BlockSpecial.cc:156)
==25110== by 0x40E7C4: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
=25110== Address 0x1231ea93 is 115 bytes inside a block of size 32,816 free'd
==25110== at 0x4C2ACBD: free (vg_replace_malloc.c:530)
==25110== by 0x9F773AC: closedir (in /usr/lib64/libc-2.17.so)
==25110== by 0x40E7AC: get_link_name (test_BlockSpecial.cc:153)
==25110== by 0x40E7AC: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
==25110== Block was alloc'd at
==25110== at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==25110== by 0x9F77280: __alloc_dir (in /usr/lib64/libc-2.17.so)
==25110== by 0x40E746: get_link_name (test_BlockSpecial.cc:134)
==25110== by 0x40E746: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
==25110== Invalid read of size 1
==25110== at 0x4C2E220: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:1022)
==25110== by 0x953A997: std::string::append(char const*, unsigned long) (in /usr/lib64/libstdc++.so.6.0.19)
==25110== by 0x40E7D2: append (basic_string.h:1009)
==25110== by 0x40E7D2: operator+<char, std::char_traits<char>, std::allocator<char> > (basic_string.h:2468)
==25110== by 0x40E7D2: get_link_name (test_BlockSpecial.cc:156)
==25110== by 0x40E7D2: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
==25110== Address 0x1231ea93 is 115 bytes inside a block of size 32,816 free'd
==25110== at 0x4C2ACBD: free (vg_replace_malloc.c:530)
==25110== by 0x9F773AC: closedir (in /usr/lib64/libc-2.17.so)
==25110== by 0x40E7AC: get_link_name (test_BlockSpecial.cc:153)
==25110== by 0x40E7AC: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
==25110== Block was alloc'd at
==25110== at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==25110== by 0x9F77280: __alloc_dir (in /usr/lib64/libc-2.17.so)
==25110== by 0x40E746: get_link_name (test_BlockSpecial.cc:134)
==25110== by 0x40E746: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
[ OK ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches (50 ms)
Selected lines from test_BlockSpecial.cc:
132 static std::string get_link_name()
133 {
134 DIR * dir = opendir( "/dev/disk/by-id" );
...
141 bool found = false;
142 struct dirent * dentry;
143 // Silence GCC [-Wparentheses] warning with double parentheses
144 while ( ( dentry = readdir( dir ) ) )
145 {
146 if ( strcmp( dentry->d_name, "." ) != 0 &&
147 strcmp( dentry->d_name, ". " ) != 0 )
148 {
149 found = true;
150 break;
151 }
152 }
153 closedir( dir );
154
155 if ( found )
156 return std::string( "/dev/disk/by-id/" ) + dentry->d_name;
So the memory referred to by dentry was allocated on line 134, freed on
153 and accessed after freed on 156.
Closes!41 - Fix test (dentry->d_name is invalidated by closedir...)
After an NTFS file system has been resized the command GParted currently
uses to read the file system usage fails like this:
# ntfsinfo --mft /dev/sdb1
Volume is scheduled for check.
Please boot into Windows TWICE, or use the 'force' option.
NOTE: If you had not scheduled check and last time accessed this volume
using ntfsmount and shutdown system properly, then init scripts in your
distribution are broken. Please report to your distribution developers
(NOT to us!) that init scripts kill ntfsmount or mount.ntfs-fuse during
shutdown instead of proper umount.
Failed to open '/dev/sdb1'.
Fix by added the '--force' flag as described in the error message.
Closes#57 - NTFS Resize results in Partition Information Warning on
Refresh
GParted uses ntfsinfo to read the NTFS file system usage. However when
ntfsinfo fails with a non-zero exit status GParted reports stdout twice,
rather than stdout and stderr. Correct this.
Closes#57 - NTFS Resize results in Partition Information Warning on
Refresh
With the same case as from the previous commit, the very long "Mounted
on ..." text is now wrapped, but the text may not be left justified.
Slowly adjust the dialog width and see how the text wrapping is updated
to fit the size adjustment but the text is centred rather than left
justified.
This is because setting the halign property to Gtk::ALIGN_START does not
guarantee left alignment of text for wrapped or ellipsized Gtk::Labels.
Use the xalign property instead.
To set the xalign property there is a method in the GtkMisc (Gtk::Misc)
base class:
gtk_misc_set_alignment (Gtk::Misc::set_alignment)
However, GtkMisc (Gtk::Misc) was deprecated in Gtk 3.14 (Gtkmm 3.14)
and in Gtk 3.16 (gtkmm 3.16) set_alignment() was replaced with the
introduction of two new methods:
gtk_label_set_xalign (Gtk::Label::set_xalign)
gtk_label_set_yalign (Gtk::Label::set_yalign)
Add a check for Gtkmm method Gtk::Label::set_xalign() in configure.ac
and use it when available.
References:
[1] Gtk3 Reference Documentation - gtk_misc_set_alignment()
https://developer.gnome.org/gtk3/stable/GtkMisc.html#gtk-misc-set-alignment
"gtk_misc_set_alignment has been deprecated since version 3.14 and
should not be used in newly-written code. Use GtkWidget's alignment
("halign" and "valign") and margin properties or GtkLabel's
"xalign" and "yalign" properties."
[2] Gtkmm 3.16 Gtk::Misc Class Reference, set_alignment() method
https://developer.gnome.org/gtkmm/3.16/classGtk_1_1Misc.html#a52b2675874cf46a3097938756b9fe9e8
[3] GNOME BugZilla - EmptyBoxes: instructions_label's alignment is off
https://bugzilla.gnome.org/show_bug.cgi?id=735841
[4] Gtk commit from 2014-09-16:
GtkLabel: add x/yalign properties
https://gitlab.gnome.org/GNOME/gtk/commit/d39424fc
[5] Gtk3 Reference Documentation - gtk_label_set_xalign()
https://developer.gnome.org/gtk3/stable/GtkLabel.html#gtk-label-set-xalign
[6] Gtkmm 3.16 Gtk::Label Class Reference, set_xalign() method
https://developer.gnome.org/gtkmm/3.16/classGtk_1_1Label.html#acee7d4e87d7cc14080a7b8ded5f84e5eCloses!40 - Limit wrapping labels
Opening the Partition Information dialog for a file system mounted on a
very long mount point, or on openSUSE which mounts the OS from 10 btrfs
subvolumes from the same partition, will cause the dialog to be very
wide as the "Mounted on ..." text is not wrapped.
Back in Gtk2, when width_chars / max_width_chars were not set, wrapping
labels had a default width beyond which text wrapped onto a new line
[1].
For Gtk3 this default width was first reworked a bit [2], and then was
removed for the very early Gtk3 3.0.10 release [3].
It is recommended that applications explicitly set default values,
otherwise wrapping labels never wrap when requesting their natural
allocation.
References:
[1] Gtk 2.24.32 source code - gtk/gtklabel.c:2975
https://gitlab.gnome.org/GNOME/gtk/blob/2.24.32/gtk/gtklabel.c#L2975
"This long string gives a good enough length for any line to
have."
[2] Gtk commit from 2010-04-21:
680d7762ba
Make sure not to base the minimum size on "max-width-chars", only
the natural size.
"This string is just about long enough."
[3] Gtk commit from 2011-04-17:
c8ce1106c1
label: Don't try to guess a label's size
People should use window default sizes or label
width-chars/max-width-chars to find the ideal layout for a label
instead of relying on magic.
Closes!40 - Limit wrapping labels
Before Gtk 3.22 GtkScrolledWindow propagated natural size to its
Children and so on to descendants. In Gtk 3.22 this was changed to
always request the minimum size. This was done because it is believed
to be a safer default (gives a better behaviour) in case of dynamic
content inside the scrolled window, that is, content that may change
allocated size. [1][2][3]
When the scrolled window content is not dynamic the natural size is
preferable because it gives a better looking layout and without any
downside.
In the case of GParted content which is not dynamic, so request the
scrolled windows to allocate children at natural sizes for Gtk >= 3.22.
The benefits of natural size allocation are evident in presence of
wrapping labels (for example inside the "Partition Info" dialog), that
with the minimum size request likely end up taking a very small width.
References:
[1] Gtk commit from 2016-08-31:
GtkScrolledWindow: Make propagation of natural child sizes optional
0984d1622d
"Making propagation of child natural sizes mandatory (or default,
even) was evidently a mistake as this causes dynamic content in a
scrolled window to resize it's parent when the scrolled window is
competing for space with an adjacent widget."
[2] Gtk 3.22 Reference Documentation -
gtk_scrolled_window_set_propagate_natural_width
https://developer.gnome.org/gtk3/3.22/GtkScrolledWindow.html#gtk-scrolled-window-set-propagate-natural-width
[3] Gtkmm 3.24 Gtk::ScrolledWindow Class Reference,
set_propagate_natural_width() method
https://developer.gnome.org/gtkmm/3.24/classGtk_1_1ScrolledWindow.html#a2d4cb945688ecb8739efd70b18742779
[4] Gtkmm 3.21.6 NEWS
https://gitlab.gnome.org/GNOME/gtkmm/blob/3.21.6/NEWS
"ScrolledWindow: Added get/set_propagate_natural_height/width() and
the properties."
Closes!39 - Always request natural size inside Gtk::ScrolledWindow
Gtk::Table was deprecated in Gtk 3.4.0. Replace with Gtk::Grid.
This commit makes the change for Dialog_Partition_Info.
Closes!25 - Modern Gtk3 - part 1
Gtk::Table was deprecated in Gtk 3.4.0. Replace with Gtk::Grid.
This commit makes the change for Dialog_Base_Partition.
Closes!25 - Modern Gtk3 - part 1
Gtk::HBox and Gtk::VBox were deprecated in Gtkmm 3.2. Replace with
plain Gtk::Box.
This commit makes the change for Dialog_Progress.cc.
Closes!25 - Modern Gtk3 - part 1
Gtk::HBox and Gtk::VBox were deprecated in Gtkmm 3.2. Replace with
plain Gtk::Box.
This commit makes the change for Dialog_Rescue_Data.cc.
Closes!25 - Modern Gtk3 - part 1
Gtk::HBox and Gtk::VBox were deprecated in Gtkmm 3.2. Replace with
plain Gtk::Box.
This commit makes the change for DialogPasswordEntry.cc.
Closes!25 - Modern Gtk3 - part 1
Gtk::HBox and Gtk::VBox were deprecated in Gtkmm 3.2. Replace with
plain Gtk::Box.
This commit makes the change for Dialog_Partition_Name.cc.
Closes!25 - Modern Gtk3 - part 1
Gtk::HBox and Gtk::VBox were deprecated in Gtkmm 3.2. Replace with
plain Gtk::Box.
This commit makes the change for Dialog_FileSystem_Label.cc.
Closes!25 - Modern Gtk3 - part 1
Gtk::HBox and Gtk::VBox were deprecated in Gtkmm 3.2. Replace with
plain Gtk::Box.
This commit makes the change for DialogFeatures.cc.
Closes!25 - Modern Gtk3 - part 1
Gtk::HBox and Gtk::VBox were deprecated in Gtkmm 3.2. Replace with
plain Gtk::Box.
This commit makes the change for Dialog_DiskLabel.cc.
Closes!25 - Modern Gtk3 - part 1
Gtk::HBox and Gtk::VBox were deprecated in Gtkmm 3.2. Replace with
plain Gtk::Box.
This commit makes the change for Dialog_Partition_Info.{h,cc}.
Closes!25 - Modern Gtk3 - part 1
Gtk::HBox and Gtk::VBox were deprecated in Gtkmm 3.2. Replace with
plain Gtk::Box.
This commit makes the change for Dialog_Base_Partition.{h,cc}.
Closes!25 - Modern Gtk3 - part 1
Gtk::HBox and Gtk::VBox were deprecated in Gtkmm 3.2. Replace with
plain Gtk::Box.
This commit makes the change for HBoxOperations.{h,cc}.
Closes!25 - Modern Gtk3 - part 1
Mike Fleetwood