... code pattern. This is to make the code easier to understand by not
having to remember if condition context for indented code over longer
distances. This has been done before. Here are just 2 examples:
[1] 75bda733bb
Refactor run_blkid_load_cache() into if fail return early (#131)
[2] 407e0ac6e3
Refactor fat16::read_label() into if fail return early pattern (!104)
Closes!119 - Tidy-ups for file system interface classes
Restructure the variable parsing code into "if leading text found then
scan the number" pattern.
Closes!119 - Tidy-ups for file system interface classes
Restructure the variable parsing code into "if leading text found then
scan the number" pattern.
Anchor leading text matches to the start of a new line in the output.
Closes!119 - Tidy-ups for file system interface classes
Restructure the variable parsing code into "if leading text found then
scan the number" pattern.
Closes!119 - Tidy-ups for file system interface classes
And restructure the variable parsing code into "if leading text found
then scan the number" pattern.
Anchor leading text matches to the start of a new line in the output.
Closes!119 - Tidy-ups for file system interface classes
FileSystem member variables T, N & S are being used like local variables
in many of the file system specific set_used_sectors() methods. They
are only used within each set_used_sectors() call and not used to
represent persistent information of a FileSystem interface class or to
pass information between separate methods. Therefore stop using them
and replace them with local variables instead.
This block of code finds a field in the output and scans the number:
Glib::ustring::size_type index = output.find("Block count:");
if (index >= output.length() ||
sscanf(output.substr(index).c_str(), "Block count: %lld", &T) != 1)
T = -1;
The if statement says "if leading text is not found or scanning the
number fails then assign -1". A sequence of two negatives leading to
assigning an error value is hard to understand. Instead this an
equivalent block from btrfs::set_used_sectors():
long long total_bytes = -1;
Glib::ustring::size_type index = output.find("\ntotal_bytes");
if (index < output.length())
sscanf(output.substr(index).c_str(), "\ntotal_bytes %lld", &total_bytes);
This assigns a default error value and the if statement says "if leading
text found then scan the number". Much simpler to understand.
Therefore change the code around to use this same pattern.
Anchor the leading text matches to the start of a new line in the
output where possible. Just because it's what some of the other file
system's set_used_sectors() methods do (btrfs, reiser4 and xfs) and it
seems like more robust text matching.
Closes!119 - Tidy-ups for file system interface classes
Replace floating point calculation to convert size and space figures
from file system block sized units to sectors with an integer
calculation. Do this for the same reasons discussed in commit "Stop
using floating point calculations in FS resize() methods" earlier in
this patchset. This will limit the largest file system that GParted can
read the usage of to 8 EiB - 1 bytes.
There is still a floating point calculation in btrfs::set_used_sectors()
which is being left because that is apportioning used space figure
between multiple devices.
Closes!119 - Tidy-ups for file system interface classes
A number of the file system specific resize() methods use floating point
calculations to convert from the new partition size in sectors to the
new file system size to be passed to the resize command in bytes or
kibibytes. This is bad because there could be rounding errors
converting from integer to floating point, performing the calculation
and converting back. Replace with integer only multiply and divide
calculations. Integer division always truncates [1] which is exactly
what is needed. The largest integer will be the size of the file system
in bytes held in a signed 64-bit long long, or Sector or Byte_Value
typedef of the same type. This will limit the size that a file system
can be shrunk to, to 8 EiB - 1 byte.
[1] C++ Arithmetic operators
https://en.cppreference.com/w/cpp/language/operator_arithmetic
"the algebraic quotient of integer division is truncated towards
zero (fractional part is discarded)"
Closes!119 - Tidy-ups for file system interface classes
The GUI displays the file system of an open encrypted file system as
"[Encrypted] FSTYPE" [1]. However saved details just writes the file
system type as "luks". Update saved details writing code to use the
same method the GUI currently uses [2].
[1] commit cb3cc505ce
Display "[Encrypted] FSTYPE" in the File System column (#760080)
[2] commit bd6fc67afb
Provide virtual Partition::get_filesystem_string() method (#774818)
Keep the paragraph discussing photorec and move just after testdisk is
mentioned in the Recovering Partition Tables section.
Closes!118 - Remove Attempt Data Rescue and use of gpart
gpart scans a drive trying to guess the location of partitions when an
MBR partition table is lost [1]. However the tool is unmaintained,
takes hours or days of 100% CPU time to scan a drive and provides no
progress indication [2][3][4]. We keep recommending killing the gpart
process and using TestDisk [5] instead.
Therefore remove Device > Attempt Data Rescue and the use of gpart from
GParted.
[1] Gpart
https://github.com/baruch/gpart
[2] Have you had a good or bad experience with Dev->Attempt Data Rescue?
http://gparted-forum.surf4.info/viewtopic.php?id=17992
No good, only bad experiences using gpart were reported.
[3] Gparted does not say anything
http://gparted-forum.surf4.info/viewtopic.php?id=17749
Forum user reported waiting 48 hours with no progress indication.
We recommended using TestDisk.
[4] How cancel Data Rescue process?
http://gparted-forum.surf4.info/viewtopic.php?id=18143
Forum user reported it will take 3 days to scan their external 480GB
drive. We recommended using TestDisk instead.
[5] TestDisk, Data Recovery
https://www.cgsecurity.org/wiki/TestDiskCloses!118 - Remove Attempt Data Rescue and use of gpart
When compiling the tests, this warning is reported:
$ make check
... warning: ...: INSTANTIATE_TEST_CASE_P is deprecated, please use INSTANTIATE_TEST_SUITE_P [-Wdeprecated-declarations]
static_assert(::testing::internal::InstantiateTestCase_P_IsDeprecated(), \
^
test_SupportedFileSystems.cc:625:1: note: in expansion of macro 'INSTANTIATE_TEST_CASE_P'
Google Test 1.10.0 release notes [1] say:
High Level Changes:
This release deprecated "....TEST_CASE" API in favor of
"....TEST_SUITE". In a nutshell if you have code that uses
something like "INSTANTIATE_TYPED_TEST_CASE_P " - this and all other
"*_TEST_CASE " are now deprecated in favor of more standard
_TEST_SUITE.
Replace the deprecated API with the new API.
[1] Google Test release v1.10.0
https://github.com/google/googletest/releases/tag/release-1.10.0Closes!117 - Require C++11 compilation
So far GParted includes Google Test 1.8.1 [1], which was the latest
release which supported pre-C++11 compilers [2]. Now that GParted
requires C++11 compilation, update to Google Test 1.10.0. Replace the
following files and directories from Google Test 1.10.0:
LICENSE
README.md
include/
src/
Note the LICENSE file is identical, where as the other files have
changed. This includes file additions and removals, hence the change
to Makefile.am too.
Even though Google Test releases up to and including 1.12.1 are
compilable with C++11 compilers [3], it is not possible to upgrade
beyond Google Test 1.10.0 at this time because later releases fail to
compile on on still supported RHEL / CentOS 7 with this error:
$ cd lib/gtest
$ make check
...
./include/gtest/gtest-matchers.h:414:12: error: 'is_trivially_copy_constructible' is not a member of 'std'
std::is_trivially_copy_constructible<M>::value &&
^
This failure turns out to be because GCC libstdc++ 4.8.5 doesn't include
is_trivially_copy_constructible et al [4][5].
[1] commit 2b222978f5
Update to Google Test 1.8.1
[2] Google Test release v1.8.1
https://github.com/google/googletest/releases/tag/release-1.8.1
"The 1.8.x is the last release supporting pre-C++11 compilers."
[3] Google Test release v1.12.1
https://github.com/google/googletest/releases/tag/release-1.12.1
"This will be the last release to support C++11. Future releases
will require at least C++14."
[4] 'is_trivially_copyable' is not a member of 'std'
https://stackoverflow.com/questions/25123458/is-trivially-copyable-is-not-a-member-of-std/25123551#25123551
"Some of them are not implemented. If we look at libstdc++'s C++11
status page:
Type properties are listed as partially implemented.
They list as missing:
...
is trivially_copy_constructible
"
[5] The GNU C++ Library, 1. Status, C++11
https://gcc.gnu.org/onlinedocs/gcc-4.8.3/libstdc++/manual/manual/status.html#status.iso.2011
"
| Section | Description | Status | Comments
...
| 20.9.4.3 | Type properties | Partial | Missing
is_trivially_copyable, is_trivially_constructible,
is_trivially_default_constructible, is_trivially_copy_constructible,
is_trivially_move_constructible, is_trivially_assignable,
is_trivially_default_assignable, is_trivially_copy_assignable,
is_trivially_move_assignable |
"
Closes!117 - Require C++11 compilation
In C++11, nullptr [1] is the strongly typed value to use instead of the
macro NULL [2]. Use everywhere [3][4].
[1] nullptr, the pointer literal (since C++11)
https://en.cppreference.com/w/cpp/language/nullptr
[2] NULL
https://en.cppreference.com/w/cpp/types/NULL
[3] Bjarne Stroustrup's C++ Style and Technique FAQ, Should I use NULL
or 0?
https://www.stroustrup.com/bs_faq2.html#null
"In C++, the definition of NULL is 0, so there is only an
aesthetic difference. I prefer to avoid macros, so I use 0.
Another problem with NULL is that people sometimes mistakenly
believe that it is different from 0 and/or not an integer. In
pre-standard code, NULL was/is sometimes defined to something
unsuitable and therefore had/has to be avoided. That's less
common these days.
If you have to name the null pointer, call it nullptr; that's
what it's called in C++11. Then, "nullptr" will be a keyword.
"
[4] What is nullptr in C++? Advantages, Use Cases & Examples
https://favtutor.com/blogs/nullptr-cpp
"Advantages of nullptr
...
Compatible: Null pointers are compatible with null pointer
constants in the C style (such as NULL and 0). This implies
that old C code that uses these constants and null pointers can
communicate with each other in C++.
"
Closes!117 - Require C++11 compilation
As discussed in the previous commit the oldest supported distributions
now provide gtkmm versions higher that 3.18.0 (which requires C++11
compilation). Therefore increase the minimum required version to gtkmm
3.18.0. This allows removal of HAVE_LABEL_SET_XALIGN autoconf
definition and associated fallback code.
Closes!117 - Require C++11 compilation
All the oldest supported distributions now have versions of GTK C++
libraries which require C++11 compilation, therefore forcing GParted to
require C++11 compilation [1][2][3]. Fragment of existing ./configure
output which lists the library versions requiring C++11 compilation:
$ ./configure
...
checking for glibmm >= 2.45.40 which requires C++11 compilation... yes
checking for libsigc++ >= 2.5.1 which requires C++11 compilation... yes
checking for gtkmm >= 3.18.0 which requires C++11 compilation... yes
checking whether g++ supports C++11 features with -std=gnu++11... yes
The oldest supported distributions and their versions of GTK C++
libraries and GCC compiler:
Distribution gtkmm glibmm libsigc++ gcc
Debian 10 3.24.0 2.58.0 2.10.1 4.8.3
RHEL / CentOS 7.9 3.22.2 2.56.0 2.10.0 4.8.5
SLES 12 SP5 3.20.1 2.48.1 2.8.0 4.8
Ubuntu 20.04 LTS 3.24.2 2.64.2 2.10.2 9.3.0
Technically GCC didn't have full C++11 support until GCC 4.8.1 [6] and
SLES 12 SP5 only has GCC 4.8 (as well as many later versions of GCC).
However which ever version of the GCC compiler is being used to compile
the GTK C++ libraries it must have all the features needed to compile
those libraries.
Simplify the configure script and just always require a C++11 capable
compiler. This also allows the use of C++11 features in the GParted
code.
[1] commit cc0740148e
port-to-gtk3: Switch to Gtkmm3 (#7)
[2] commit 707bae6fed
Enable C++11 compilation when using libsigc++ 2.5.1 and later (#758545)
[3] commit d6d7cb2bbf
Enable C++11 compilation when using glibmm 2.45.40 and later (#756035)
[4] SUSE Product Support Lifecycle, SUSE Linux Enterprise Server 12
https://www.suse.com/lifecycle/#suse-linux-enterprise-server-12
[5] SUSE package search
https://scc.suse.com/packages?name=SUSE%20Linux%20Enterprise%20Server&version=12.5&arch=x86_64&query=&module=
[6] C++ Standards Support in GCC, C++11 Support in GCC
https://gcc.gnu.org/projects/cxx-status.html#cxx11Closes!117 - Require C++11 compilation
When blanking of udev rules was first tested [1][2] and added [3] all
the distributions at the time (CentOS 6, Debian 6, Fedora 19,
openSUSE 12.2, Ubuntu 12.04 LTS) stored the system default rules in
directory /lib/udev/rules.d. Now most distributions (CentOS Stream 9,
Debian 11, Fedora 38, Ubuntu 22.04 LTS, openSUSE Leap 15.4) store the
system default rules in directory /usr/lib/udev/rules.d. Most of these
distributions have a merged /usr file system [4][5] so /lib is a symlink
to /usr/lib and the system default rules can still found using the
original directory. But openSUSE 15.4 doesn't have a merged /usr so the
gparted shell wrapper doesn't find the system default rules in directory
/usr/lib/udev/rules.d and doesn't prevent auto starting of Linux
Software RAID arrays and bcache devices during a storage probe.
An extra consideration is that Alpine Linux 3.17 doesn't have a merged
/usr file system, but has both /lib/udev/rules.d and
/usr/lib/udev/rules.d directories with different rules files. Therefore
fix this by checking for system default udev rules in both directories.
[1] Bug 709640 - Linux Swap Suspend and Software RAID partitions not
recognised, comment 7
https://bugzilla.gnome.org/show_bug.cgi?id=709640#c7
[2] Bug 709640 - Linux Swap Suspend and Software RAID partitions not
recognised, comment 12
https://bugzilla.gnome.org/show_bug.cgi?id=709640#c12
[3] a255abf343
Prevent GParted starting stopped Linux Software RAID arrays (#709640)
[4] The Case for the /usr Merge
http://0pointer.de/blog/projects/the-usr-merge
[5] The Case for the /usr Merge
https://www.freedesktop.org/wiki/Software/systemd/TheCaseForTheUsrMerge/Closes!116 - Systemd mount masking and udev rule location updates
After the previous commit "Stop masking the root file system mount
unit", GParted now reports this error to the terminal on Debian 10 and
11:
# gparted
To few arguments.
GParted 1.5.0-git
configuration --enable-online-resize
libparted 3.2
Debian installations, at least on PC hardware and using BIOS booting so
they don't have a /boot/efi file system, only have a single file system,
/ (root). That is now excluded from masking so gparted shell wrapper
runs systemctl without any mount units to mask. Hence the error.
# systemctl --runtime mask --quiet --
Too few arguments.
# echo $?
1
Fix this by only masking and unmasking units when the list of mounts
is non-empty.
Closes!116 - Systemd mount masking and udev rule location updates
Masking the root file system (-.mount) unit lead to a Debian package
upgrade failing as reported here [1]. This was fixed in systemd 245
[2][3] by not allowing perpetual units to be masked. As the root file
system can't be mounted or unmounted while GParted is running, it
doesn't need to be prevented by masking the unit. Therefore stop
masking the root file system mount unit.
[1] Debian bug #948710 - handle masked .mount unit more gracefully
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=948710
[2] systemd issue #14550 - Handle masked .mount units more gracefully
https://github.com/systemd/systemd/issues/14550
[3] core: never allow perpetual units to be masked
88414eed6fCloses!116 - Systemd mount masking and udev rule location updates
On RHEL / CentOS 8 GParted reports this error to the terminal when it is
closed:
# gparted
GParted 1.5.0-git
configuration --enable-online-resize
libparted 3.2
>> --runtime cannot be used with unmask
# $?
0
and leaves mount units masked:
# systemctl list-units '*.mount'
UNIT LOAD ACTIVE SUB DESCRIPTION
------------------------------------------------------------------
* -.mount masked active mounted Root Mount
* boot.mount masked active mounted boot.mount
...
This is because of this change [1] released in systemd 239. Systemd bug
9393 [2] was raised and the change was reverted [3] in systemd 240.
According to repology.org only RHEL / CentOS 8 (and clones) and Fedora
29 shipped with systemd 239 [4].
Fix by detecting non-zero exit status from systemctl and falling back to
directly removing the runtime mount unit mask files instead. Then have
to use systemctl daemon-reload to inform systemd to reload it's
configuration from disk to discover the masks have been removed.
[1] systemctl: when removing enablement or mask symlinks, cover both
/run and /etc
4910b35078
[2] systemctl no longer allows unmask in combination with --runtime
#9393https://github.com/systemd/systemd/issues/9393
[3] Revert "systemctl: when removing enablement or mask symlinks, cover
both /run and /etc"
1830ac51a4
[4] Versions for systemd
https://repology.org/project/systemd/versionsCloses!116 - Systemd mount masking and udev rule location updates
Mike Fleetwood