Initialise capture string with sacrificial text before each test.
Existing tests confirm this is cleared first by checking the captured
string matches the input string.
Bug 777973 - Segmentation fault on bad disk
Seems more logical to initially clear the output capture buffer in a
single location in the PipeCapture class which reads the command output
into said buffer, rather than each calling site before the PipeCapture
objects are constructed.
Bug 777973 - Segmentation fault on bad disk
A user had a very corrupted FAT file system and when GParted was reading
the file system usage it would core dump. The tip of the stack trace
was:
#0 Glib::ustring_Iterator<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::operator++()
at /usr/include/glibmm-2.4/glibmm/ustring.h line 957
#1 Glib::ustring_Iterator<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::operator++(int)
at /usr/include/glibmm-2.4/glibmm/ustring.h line 965
#2 GParted::PipeCapture::OnReadable(Glib::IOCondition)
at PipeCapture.cc line 63
#3 GParted::PipeCapture::_OnReadable(_GIOChannel*, GIOCondition, void*)
at PipeCapture.cc line 45
GParted uses 'fsck.fat -n -v /dev/PTN' to get the file system usage.
However because the file system was so corrupted it was reporting every
file name as being damaged, including the file names being reported as
binary data. PipeCapture::OnReadable() reads up to 512 bytes at a time
and then uses a Glib::ustring iterator to loop over the UTF-8
characters, but a Glib::ustring iterator is explicitly not capable of
reading binary data [1]. With invalid UTF-8 bytes the code continued to
read beyond the end of the string until GParted crashed with a
segmentation violation.
Fix by accessing the read string by index instead of by iterator.
[1] Quote from the Glib::ustring_Iterator<T> Class Template Reference:
https://developer.gnome.org/glibmm/stable/classGlib_1_1ustring__Iterator.html
"The Glib::ustring iterated over must contain only valid UTF-8 data.
If it does not, operator++(), operator--() and operator*() may make
accesses outside the bounds of the string."
Bug 777973 - Segmentation fault on bad disk
A user had a very corrupt FAT file system and fsck.fat was reporting
binary data in file names with errors. GParted crashed reading this
output. Boiled down the problematic fsck.fat output to a sample 27
bytes which still triggers a crash and created a test for it. The test
crashes test_PipeCapture program too.
$ ./test_PipeCapture
...
[ RUN ] PipeCaptureTest.MinimalBinaryCrash777973
Segmentation fault (core dumped)
$ echo $?
139
However it still produces a non-zero exit status and the autotools test
runner detects this so 'make check' still reports failure.
$ make check
...
make[2]: Entering directory `/home/centos/programming/c/gparted/tests'
PASS: test_dummy
PASS: test_BlockSpecial
../test-driver: line 95: 16152 Segmentation fault "$@" > $log_file 2>&1
FAIL: test_PipeCapture
...
============================================================================
Testsuite summary for gparted 0.28.1-git
============================================================================
# TOTAL: 3
# PASS: 2
# SKIP: 0
# XFAIL: 0
# FAIL: 1
# XPASS: 0
# ERROR: 0
============================================================================
See tests/test-suite.log
Please report to https://bugzilla.gnome.org/enter_bug.cgi?product=gparted
============================================================================
make[2]: *** [test-suite.log] Error 1
make[2]: Leaving directory `/home/centos/programming/c/gparted/tests'
make[1]: *** [check-TESTS] Error 2
make[1]: Leaving directory `/home/centos/programming/c/gparted/tests'
make: *** [check-am] Error 2
$ echo $?
2
Bug 777973 - Segmentation fault on bad disk
Also ensure that the PipeCapture calls registered update callbacks and
that the data so far captured matches the leading portion of the input.
Bug 777973 - Segmentation fault on bad disk
Google Test string comparison asserts are only designed of C style
strings containing printable text of one or more lines with a
terminating NUL character. GParted is crashing when PipeCapture is
reading the binary file names being reported by fsck.fat from a very
corrupted FAT file system. Therefore need to be able to compare and
report differences of binary data stored in C++ std::string and
Glib::ustrings. Write a specific assertion to handle this.
Now these sample tests:
TEST_F( PipeCaptureTest, BinaryStringFailure )
{
inputstr = "AAAAAAAAAAAAAAAABBBBBBBBBBBBBBBBCCCCCCCCCCCCCCCC";
capturedstr = "AAAAAAAAAAAAAAAABBBBBBBBBBbbbb";
EXPECT_BINARYSTRINGEQ( inputstr, capturedstr.raw() );
}
TEST_F( PipeCaptureTest, LeadingBinaryStringFailure )
{
inputstr = "The quick brown fox jumps over the lazy dog";
capturedstr = "The quick brown fox\n";
EXPECT_BINARYSTRINGEQ( inputstr.substr( 0, capturedstr.raw().length() ),
capturedstr.raw() );
}
report failure like this:
$ ./test_PipeCapture
...
[ RUN ] PipeCaptureTest.BinaryStringFailure
test_PipeCapture.cc:270: Failure
Expected: inputstr
Of length: 48
To be equal to: capturedstr.raw()
Of length: 30
With first binary difference:
< 0x00000010 "BBBBBBBBBBBBBBBB" 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42
--
> 0x00000010 "BBBBBBBBBBbbbb" 42 42 42 42 42 42 42 42 42 42 62 62 62 62
[ FAILED ] PipeCaptureTest.BinaryStringFailure (1 ms)
[ RUN ] PipeCaptureTest.LeadingBinaryStringFailure
test_PipeCapture.cc:278: Failure
Expected: inputstr.substr( 0, capturedstr.raw().length() )
Of length: 20
To be equal to: capturedstr.raw()
Of length: 20
With first binary difference:
< 0x00000010 "fox " 66 6F 78 20
--
> 0x00000010 "fox." 66 6F 78 0A
[ FAILED ] PipeCaptureTest.LeadingBinaryStringFailure (0 ms)
...
Bug 777973 - Segmentation fault on bad disk
Add test sending 1 MiB of ASCII test into PipeCapture to read. This
requires multiple reads and rewites through the pipe.
This is as much a check of the threading in the PipeCaptureTest class as
it is of PipeCapture itself. This is because a single thread might
block reading from a pipe waiting for itself to write to the pipe.
Deadlock. Hence the requirement to use a separate thread for reading
and writing.
Bug 777973 - Segmentation fault on bad disk
On CentOS 6, with glib/glibmm 2.28, running the tests fails thus:
$ ./test_PipeCapture
Running main() from gtest_main.cc
[==========] Running 4 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 4 tests from PipeCaptureTest
[ RUN ] PipeCaptureTest.EmptyPipe
GLib-ERROR **: The thread system is not yet initialized.
aborting...
Aborted (core dumped)
For glib before version 2.32, the threading system must be explicitly
initialised with a call to g_thread_init(), or the Glibmm
Glib::thread_init() equivalent to prevent this error.
Deprecated thread API, g_thread_init()
https://developer.gnome.org/glib/stable/glib-Deprecated-Thread-APIs.html#g-thread-init
Do this by providing our own main() which also initialises the Glib
threading system, rather using and linking in the Google Test provided
main().
Bug 777973 - Segmentation fault on bad disk
So far just tests sending 0 bytes and a few ASCII bytes into a pipe and
that they are read correctly by the PipeCapture class.
Bug 777973 - Segmentation fault on bad disk
As the source code is managed in GIT and there is a .gitignore file in
the top level directory specifying file names to exclude from version
control, then the old per-directory .cvsignore files for CVS are
redundant.
Add the only missing and applicable entry from src/.cvsignore of '.libs'
to .gitignore and remove all the .cvsignore files.
Running ./autogen.sh reports this suggestion:
libtoolize: Consider adding `-I m4' to ACLOCAL_AMFLAGS in Makefile.am.
Add suggested setting as it causes no difference and silences the
suggestion.
The Libtool Manual, 5.5.1 Invoking libtoolize
https://www.gnu.org/software/libtool/manual/html_node/Invoking-libtoolize.html
"If libtoolize detects an explicit call to AC_CONFIG_MACRO_DIRS (see
The Autoconf Manual in The Autoconf Manual) in your configure.ac, it
will put the Libtool macros in the specified directory.
In the future other Autotools will automatically check the contents
of AC_CONFIG_MACRO_DIRS, but at the moment it is more portable to
add the macro directory to ACLOCAL_AMFLAGS in Makefile.am, which is
where the tools currently look. If libtoolize doesn't see
AC_CONFIG_MACRO_DIRS, it too will honour the first '-I' argument in
ACLOCAL_AMFLAGS when choosing a directory to store libtool
configuration macros in. It is perfectly sensible to use both
AC_CONFIG_MACRO_DIRS and ACLOCAL_AMFLAGS, as long as they are kept
in synchronisation.
ACLOCAL_AMFLAGS = -I m4
"
Deliberately breaking one of the operator<() tests produces less than
ideal diagnostics because it doesn't print the BlockSpecial objects
being compared.
$ ./test_BlockSpecial
...
[ RUN ] BlockSpecialTest.OperatorLessThanTwoEmptyObjects
test_BlockSpecial.cc:362: Failure
Value of: bs1 < bs2
Actual: true
Expected: false
[ FAILED ] BlockSpecialTest.OperatorLessThanTwoEmptyObjects (1 ms)
...
This could be solved by using the Google Test Binary Comparison
assertions, however in the tests for false from (bs1 == bs2) and
(bs1 < bs2) comparisons then operators != and >= would have to be
implemented and the tests changed from:
EXPECT_FALSE( bs1 < bs2 );
to:
EXPECT_GE( bs1, bs2 );
This makes the meaning of the test less than clear. The primary purpose
of the test is to check operator<(), but it is expecting the first
BlockSpecial object to be GE (greater than or equal to) than the second,
which is calling operator>=() which in turn is testing operator<(). For
tests of the operators themselves using Google Test Binary Comparison
assertions obscures what is being tested too much.
Instead provide a custom failure message which prints the BlockSpecial
objects failing the comparison, leaving the test still directly calling
the operator being tested, like this:
EXPECT_FALSE( bs1 < bs2 ) << " Where: bs1 = " << bs1 << "\n"
<< " And: bs2 = " << bs2;
And with a suitable macro this is simplified to:
EXPECT_FALSE( bs1 < bs2 ) << ON_FAILURE_WHERE( bs1, bs2 );
Now the above deliberately broken test produces this output:
$ ./test_BlockSpecial
...
[ RUN ] BlockSpecialTest.OperatorLessThanTwoEmptyObjects
test_BlockSpecial.cc:369: Failure
Value of: bs1 < bs2
Actual: true
Expected: false
Where: bs1 = BlockSpecial{"",0,0}
And: bs2 = BlockSpecial{"",0,0}
[ FAILED ] BlockSpecialTest.OperatorLessThanTwoEmptyObjects (0 ms)
...
Bug 781978 - Add Google Test C++ test framework
Add a specific test for the failure found in bug 771670 and fixed by
commit:
253c4d6416
Fix BlockSpecial comparison (#771670)
Bug 781978 - Add Google Test C++ test framework
So far only includes tests of the construction of BlockSpecial objects
and the interaction with the internal cache used to reduce the number of
stat(2) calls. Tests have been designed to be runable by non-root users
and assume as little as possible about the environment by discovering
used block device names and symbolic link name.
Bug 781978 - Add Google Test C++ test framework
Running 'make distcheck' fails like this:
[...]
make[3]: Entering directory `[...]/gparted/gparted-0.28.1-git/_build/tests'
[...]
/bin/sh ../libtool --tag=CXX --mode=link g++ [...] ../../lib/gtest/lib/libgtest_main.la ../../lib/gtest/lib/libgtest.la [...]
libtool: link: cannot find the library `../../lib/gtest/lib/libgtest_main.la' or unhandled argument `../../lib/gtest/lib/libgtest_main.la'
make[3]: *** [test_dummy] Error 1
make[3]: Leaving directory `[...]/gparted/gparted-0.28.1-git/_build/tests'
[...]
make[1]: Leaving directory `[...]/gparted/gparted-0.28.1-git/_build'
make: *** [distcheck] Error 1
What is happening is that distcheck [1] performs a VPATH build which
uses separate read-only source and read-write build trees. Shipped
source files, such as .h and .cc, are located in the source tree and the
built files, such as .o and .la, are compiled into the build tree. In
this case the absolute path to libgtest_main.la is specified using
$(top_srcdir) for the source tree, rather than $(top_builddir) for the
build tree, hence the cannot find file type error. This error doesn't
occur when just performing a 'make check' because that builds in the
source tree so $(top_builddir) is the same as $(top_srcdir).
Fix by correctly specifying the absolute path to the libgtest*.la files
using $(top_builddir).
[1] Automake Manual, 14.4 Checking the Distribution
https://www.gnu.org/software/automake/manual/automake.html#Checking-the-Distribution
Note that for VPATH builds the Automake Manual [2] also recommends using
a pair of -I options to specify header files are found in both the build
and source trees, something like:
AM_CPPFLAGS = -I$(top_builddir)/some/subdir -I$(top_srcdir)/some/subdir
This is unnecessary for GParted as all header files are shipped as part
of the source archive and no header files are created as part of the
build process. Therefore adding -I options specifying $(top_builddir)
include directories would just be specifying empty or non-existent
include directories in the build tree.
[2] Automake Manual, 8.7 Variables used when building a program
https://www.gnu.org/software/automake/manual/automake.html#Program-Variables
Bug 781978 - Add Google Test C++ test framework
Now 'make check' will additionally build and run the test suites in the
./tests directory. Add initial always successful dummy test suite.
This is done using Automake support for testing.
Automake Manual, 15 Support for test suites
https://www.gnu.org/software/automake/manual/automake.html#Tests
./tests/Makefile.am takes some influence from the same file in the
minimal-gtest-autotools template project.
654848ec01/tests/Makefile.am
Bug 781978 - Add Google Test C++ test framework
This closely follows the minimal-gtest-autotools template project.
https://github.com/octol/minimal-gtest-autotools
Now 'make check' will additionally build the Google Test C++ test
framework libraries.
Bug 781978 - Add Google Test C++ test framework
Running 'make check' is failing, reporting that 2 of the Google Test
files need to be translated.
$ make check
...
Making check in po
make[1]: Entering directory `/home/centos/programming/c/gparted/po'
rm -f missing notexist
srcdir=. /usr/bin/intltool-update -m
The following files contain translations and are currently not in use. Please
consider adding these to the POTFILES.in file, located in the po/ directory.
lib/gtest/include/gtest/internal/gtest-filepath.h
lib/gtest/src/gtest.cc
If some of these files are left out on purpose then please add them to
POTFILES.skip instead of POTFILES.in. A file 'missing' containing this list
of left out files has been written in the current directory.
Please report to https://bugzilla.gnome.org/enter_bug.cgi?product=gparted
if [ -r missing -o -r notexist ]; then \
exit 1; \
fi
make[1]: *** [check] Error 1
make[1]: Leaving directory `/home/centos/programming/c/gparted/po'
make: *** [check-recursive] Error 1
'make check' is failing
This is not true, the files do not contain translatable strings. This
is a match for known intltool bug:
False positives from intltool-update -m
https://bugs.launchpad.net/intltool/+bug/545862
Fix by adding the problem file names into POTFILES.skip.
Bug 781978 - Add Google Test C++ test framework
Copy the Google Test C++ test framework (subset needed to build the unit
testing libraries) into the GParted project. Copy the following files
and directories from Google Test, release 1.8.0:
LICENSE
README.md
include/
src/
into lib/gtest/. All the files are under the BSD license as documented
in lib/gtest/LICENSE.
Bug 781978 - Add Google Test C++ test framework
Attempting to grow an already mounted xfs where the mount point
contained spaces failed like this:
Grow /dev/sdb4 from 1.00 GiB to 1.50 GiB
+ calibrate /dev/sdb4
+ grow partition from 1.00 GiB to 1.50 GiB
+ grow file system to fill the partition
+ xfs_growfs /tmp/File System Label
Usage: xfs_growfs [options] mountpoint
...
Apply the rule and quote all uses of mount points within command lines.
This is also applied to copying xfs file systems even though it was safe
because it only ever used GParted generated mount points.
Also for the xfs copy operation switch unmounting of partitions to
specify mount points instead of partitions. This is just to be
consistent with how it is done in all the online file system resizing
code.
Bug 782681 - btrfs partitions mounted with whitespace cannot be resized
The current nilfs2 resizing code is safe because it never passes a user
influenced mount point into a command line. Regardless, apply the same
simple rule to always quote mount points when used in command lines.
WARNING:
Nilfs-resize is broken and can't actually resize a file system mounted
with spaces in the mount point anyway!
# mkdir "/tmp/File System Label"
# mount -v -t nilfs2 /dev/sdb3 "/tmp/File System Label"
mount.nilfs2: started nilfs_cleanerd
# nilfs-resize -v -y /dev/sdb3
Error: cannot open NILFS on /dev/sdb3.
# echo $?
1
# strace nilfs-resize -v -y /dev/sdb3
...
stat("/dev/sdb3", {st_mode=S_IFBLK|0660, st_rdev=makedev(8, 19), ...}) = 0
open("/dev/sdb3", O_RDONLY) = 3
...
open("/proc/mounts", O_RDONLY) = 4
read(4, "sysfs /sys sysfs rw,seclabel,nos"..., 1024) = 1024
...
close(4) = 0
open("/tmp/File\\040System\\040Label", O_RDONLY) = -1 ENOENT (No such file or directory)
...
# fgrep /dev/sd /proc/mounts
/dev/sda3 / ext4 rw,seclabel,relatime,data=ordered 0 0
/dev/sda1 /boot ext4 rw,seclabel,relatime,data=ordered 0 0
/dev/sdb3 /tmp/File\040System\040Label nilfs2 rw,relatime 0 0
So it looks like nilfs-resize (or a library it uses) can't decode the
octal characters '\040' used to encode spaces in the mount point as
reported in /proc/mounts.
Bug 782681 - btrfs partitions mounted with whitespace cannot be resized
Attempting to grow an already mounted jfs where the mount point
contained spaces failed like this:
Grow /dev/sdb2 from 1.00 GiB to 1.50 GiB
+ calibrate /dev/sdb2
+ grow partition from 1.00 GiB to 1.50 GiB
+ grow file system to fill the partition
+ mount -v -t jfs -o remount,resize /dev/sdb2 /tmp/File System Label
Usage:
mount [-lhV]
mount -a [options]
mount [options] [--source[ <source> | [--target] <directory>
mount [options] <source> <directory>
mount <operation> <mountpoint> [<target>]
...
Apply the rule to always enclose mount points within double quotes
within command lines.
Bug 782681 - btrfs partitions mounted with whitespace cannot be resized
A user had a btrfs file system mounted by automounter on a mount point
like "/mount/$USER/File System Label" which included white space
characters. Resizing the file system while online failed like this:
Grow /dev/sdb1 from 1.00 GiB to 1.50 GiB
+ calibrate /dev/sdb1
+ grow partition from 1.00 GiB to 1.50 GiB
+ grow file system to fill the partition
+ btrfs filesystem resize 1:max /mount/USER/File System Label
btrfs filesystem resize: too many arguments
usage: btrfs filesystem resize [devid:][+/-]<newsize>[kKmMgGtTpPeE]|[devid:]max <path>
So mount points not created by GParted should be considered under user
control and need quoting when used as parameters in command lines.
Strictly speaking, mount points created by GParted itself, by
FileSystem::mk_temp_dir(), are safe and don't need quoting. However it
is simpler and safer just to quote all uses of mount points in command
lines, rather than risk missing some.
Bug 782681 - btrfs partitions mounted with whitespace cannot be resized
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"
A regression which prevented growing a primary partition that had
unallocated space between it and the following extended partition was
introduced with the following commit:
Create and use general find_extended_partition() function
aa98107706
To fix the regression, restore the logic that checked for a logical
partition before seeking the index of the extended partition.
Bug 778700 - Unable to grow partition even though unallocated space is
adjacent
The command 'make distcheck' runs a build in dist directory subdirs
and then runs intltool -m which in turn complains about translations
in a built file:
The following files contain translations and are currently not in
use. Please consider adding these to the POTFILES.in file, located
in the po/ directory.
sub/gparted.desktop.in
See also upstream intltool issue bug report:
intltool confused by separate build-dir
https://bugs.launchpad.net/intltool/+bug/1117944
Bug 778628 - Work around automake-1.15 & intltool complaining about
translations in build dir
Increase the size of the Name Partition dialog, matching the change made
to the Label File System dialog in the previous commit. The code for
the Name Partition dialog was basically copied from the Label File
System dialog.
Bug 778003 - The "Label File System" dialog is too small
On Arch Linux with XFCE 4.12 and Fedora 24 with GNOME 3.20 and later;
the Label File System dialog is too small. The problem is that the
label entry box clips the Cancel and OK buttons.
Stop specifying the dialog height, instead letting it fit the combined
height of all the widgets automatically.
Also make the dialog wider and the label entry box wider so that longer
device names can be shown in the title before they are truncated.
Bug 778003 - The "Label File System" dialog is too small
Mike Fleetwood