Was attempting to erase the wrong key "key-long" for the test. It
should be erasing "key-too-long". Fix this. Given that that line in
the test was to erase a non-existent key and confirm failure; the test
passed before with the wrong non-existent key, and still passes with the
right non-existent key. Clearly a cut and paste error as a result of
copying the previous LongPassword test when initially added in:
c6657aab9e
Add unit tests for PasswordRAMStore module (#795617)
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...)
Use of Glib::thread_init() was deprecated in glibmm 2.32 [1]. The
oldest supported distributions have these versions:
Debian 8 glibmm 2.42.0
RHEL / CentOS 7 glibmm 2.56.0
SLES 12 glibmm 2.38.1
Ubuntu 14.04 LTS glibmm 2.39.93
Checking further the glibmm 2.32 reference manual says this about
Glib::thread_init() [2]:
Initializes the GLib thread system.
Deprecated:
Calling thread_init() is no longer necessary and no longer has any
effect.
However only some of the glibmm example programs had Glib::thread_init()
removed, others had it replaced by Glib::init() [3]. Again the glibmm
2.32 reference manual says this about Glib::init() [4]:
Initialize glibmm.
You may call this more than once. You do not need to call this if
you are using Glib::MainLoop or Gtk::Main, because they call it for
you.
GParted does call Gtk::Main and test_PipeCapture does call
Glib::MainLoop. Therefore just raise the minimum version to glibmm 2.32
and remove both calls to Glib::thread_init().
[1] Glibmm 2.32 NEWS file
https://gitlab.gnome.org/GNOME/glibmm/blob/2.32.0/NEWS#L207
[2] glibmm 2.32, glibmm: Glib Namespace Reference, Glib::thread_init()
https://developer.gnome.org/glibmm/2.32/namespaceGlib.html#ab26d01c776801f1fff00753e97af4fc7
[3] glibmm commit "Avoid use of deprecates API in tests and examples."
3e0fbb22c0
[4] glibmm 2.32, glibmm: Glib Namespace Reference, Glib::init()
https://developer.gnome.org/glibmm/2.32/namespaceGlib.html#ac90aee10d0b90e3d8a96a86b5394f87bCloses!22 - Increase minimums to libparted 2.2 and glibmm 2.32
Replace the insert() method (which reports an error when inserting a
password with a key which already exists) with the store() method which
replaces or inserts the password depending on whether the key already
exists or not respectively. There is also an optimisation that nothing
is changed if the password to be replaced is the same as the one already
stored. The code in Win_GParted::open_encrypted_partition() is
simplified now it doesn't have to implement this pattern of behaviour
itself.
Bug 795617 - Implement opening and closing of LUKS mappings
Use private access into the PasswordRAMStore class to directly obtain
the address of the locked memory, rather than inferring it from the
address of the first stored password. This simplifies
PasswordRAMStoreTest::SetUpTestCase() and avoids encoding most of the
implementation knowledge that the first password will be stored at the
start of the protected memory.
Bug 795617 - Implement opening and closing of LUKS mappings
Test that all passwords are zeroed by PasswordRAMStore::erase_all(), the
same method as used in the PasswordRAMStore destructor.
Bug 795617 - Implement opening and closing of LUKS mappings
As noted in comments:
1) This is white box testing because it uses implementation knowledge
to look through the API to the internals of the password store.
2) It is not currently possible to test that the passwords are zeroed
when the store is destroyed.
However zeroing of memory is being tested when individual passwords
are erased.
Bug 795617 - Implement opening and closing of LUKS mappings
Found that older but still supported distributions Debian 8 and
Ubuntu 14.04 LTS don't have directory /dev/disk/by-path/. This is used
by the BlockSpecial unit test as a source of a symbolic link to a block
special device.
This causes the unit test to fail like this:
$ cd tests
$ ./test_BlockSpecial
...
[ RUN ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches
test_BlockSpecial.cc:137: Failure
Failed
get_link_name(): Failed to open directory '/dev/disk/by-path'
test_BlockSpecial.cc:168: Failure
Failed
follow_link_name(): Failed to resolve symbolic link ''
test_BlockSpecial.cc:255: Failure
Expected: (lnk.m_name.c_str()) != (bs.m_name.c_str()), actual: "" vs ""
[ FAILED ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches (0 ms)
...
[ FAILED ] 1 test, listed below:
[ FAILED ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches
1 FAILED TEST
Which in turn causes make check and make distcheck to fail.
Use directory /dev/disk/by-id/ instead as it always exists.
Make distcheck is failing like:
<snip>
Make distcheck
Making distclean in tests
make[2]: Entering directory
'/home/gedakc/workspace/gparted/gparted-0.29.0-beta1/_build/sub/tests'
Makefile:658: ../src/.deps/BlockSpecial.Po: No such file or directory
Makefile:659: ../src/.deps/PipeCapture.Po: No such file or directory
make[2]: *** No rule to make target '../src/.deps/PipeCapture.Po'. Stop.
make[2]: Leaving directory
'/home/gedakc/workspace/gparted/gparted-0.29.0-beta1/_build/sub/tests'
Makefile:596: recipe for target 'distclean-recursive' failed
make[1]: *** [distclean-recursive] Error 1
make[1]: Leaving directory
'/home/gedakc/workspace/gparted/gparted-0.29.0-beta1/_build/sub'
Makefile:805: recipe for target 'distcheck' failed
make: *** [distcheck] Error 1
Make distcheck started failing like this with this commit:
f31808989a
Silence subdir-objects warning from automake 1.14 (#781978)
However, also need to be referring to source files from a sibling
directory like this from tests/Makefile.am:
test_BlockSpecial_SOURCES = test_BlockSpecial.cc ../src/BlockSpecial.cc
^^^^^^^
First introduced by this commit:
c37be28148
Add unit tests for BlockSpecial constructors and internal caching (#781978)
This failure, if not exactly common, has been seen a number of times
before. Using this hint from a GNU Automake forum post:
converting to subdir-objects
http://gnu-automake.7480.n7.nabble.com/converting-to-subdir-objects-td21724.html
"I had the same issue and wouldn't care unless Automake 1.15 started
to warn about "source file in a subdirectory". In my case 'bar.o' is
also built from 'foo/Makefile' so I decided to skip using 'bar.c' as
SOURCE and instead link in 'foo/bar.o'.
http://dev.thep.lu.se/svndigest/changeset/1581
Cheers,
Peter
"
Fix by avoiding referring to source files from a sibling directory when
building the unit test programs, instead refer to object files instead.
Note that the automake subdir-objects option is still required to
silence the warning because of the use of source files in subdirectories
by lib/gtest/Makefile.am.
Bug 785308 - make distcheck fails with *.Po: No such file or directory
Test that binary data that happens to be the start of a multi-byte UTF-8
character but then contains a NUL byte is successfully read. The test
currently detects failure thus:
$ ./test_PipeCapture
...
[ RUN ] PipeCaptureTest.ReadNULByteInMiddleOfMultiByteUTF8Character
test_PipeCapture.cc:346: Failure
Expected: expectedstr
Of length: 7
To be equal to: capturedstr.raw()
Of length: 0
With first binary difference:
< 0x00000000 "._45678" 00 5F 34 35 36 37 38
--
> 0x00000000 ""
[ FAILED ] PipeCaptureTest.ReadNULByteInMiddleOfMultiByteUTF8Character (0 ms)
...
Bug 777973 - Segmentation fault on bad disk
Test currently detects failure thus:
$ ./test_PipeCapture
...
[ RUN ] PipeCaptureTest.ReadEmbeddedNULCharacter
unknown file: Failure
C++ exception with description "std::bad_alloc" thrown in the test body.
[ FAILED ] PipeCaptureTest.ReadEmbeddedNULCharacter (31917 ms)
...
Bug 777973 - Segmentation fault on bad disk
PipeCapture::OnReadable() has been almost completely re-written but this
test is still failing thus:
$ ./test_PipeCapture
...
[ RUN ] PipeCaptureTest.MinimalBinaryCrash777973
test_PipeCapture.cc:313: Failure
Expected: inputstr
Of length: 27
To be equal to: capturedstr.raw()
Of length: 26
With first binary difference:
< 0x00000010 "...!......." A9 C2 A0 21 E2 95 9F E2 88 A9 C2
--
> 0x00000010 "...!......" A9 C2 A0 21 E2 95 9F E2 88 A9
[ FAILED ] PipeCaptureTest.MinimalBinaryCrash777973 (0 ms)
...
The OnReadable() code specifically skips invalid bytes which aren't part
of valid UTF-8 characters, because they can't be displayed to the user.
The final C2 byte is the start of a multi-byte UTF-8 character, so on
it's own is invalid. Therefore PipeCapture skips it. Update expected
string accordingly. Now the test passes.
Bug 777973 - Segmentation fault on bad disk
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
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
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
Mike Fleetwood