[libcxx] r273078 - Fix 3 bugs in filesystem tests and implementation.
Eric Fiselier via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 17 21:10:23 PDT 2016
Author: ericwf
Date: Fri Jun 17 23:10:23 2016
New Revision: 273078
URL: http://llvm.org/viewvc/llvm-project?rev=273078&view=rev
Log:
Fix 3 bugs in filesystem tests and implementation.
This patch fixes the following bugs, all of which were discovered while
testing a 32 bit build on a 64 bit machine.
* path.itr/iterator.pass.cpp has undefined behavior.
'path::iterator' stashes the value of the element inside the iterator.
This violates the BiDirIterator requirements but is allowed for path::iterator.
However this means that using reverse_iterator<path::iterator> has undefined
behavior because it assumes that 'Iter tmp = it; return *tmp' will not create
a dangling reference. However it does, and this caused this particular test
to fail.
* path.native.obs/string_alloc.pass.cpp tested the SSO with a long string.
On 32 bit builds std::wstring only has the SSO for strings of size 2. The
test was using a string of size 4.
* fs.op.space/space.pass.cpp had overflows while calculating the expected values.
The fix here is to convert the statvfs data members to std::uintmax_t before
multiplying them. The internal implementation already does this but the tests
needed to do it as well.
Modified:
libcxx/trunk/test/std/experimental/filesystem/class.path/path.itr/iterator.pass.cpp
libcxx/trunk/test/std/experimental/filesystem/class.path/path.member/path.native.obs/string_alloc.pass.cpp
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.space/space.pass.cpp
libcxx/trunk/test/support/filesystem_test_helper.hpp
Modified: libcxx/trunk/test/std/experimental/filesystem/class.path/path.itr/iterator.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/class.path/path.itr/iterator.pass.cpp?rev=273078&r1=273077&r2=273078&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/class.path/path.itr/iterator.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/class.path/path.itr/iterator.pass.cpp Fri Jun 17 23:10:23 2016
@@ -24,8 +24,6 @@
#include <type_traits>
#include <cassert>
-#include <iostream>
-
#include "test_macros.h"
#include "filesystem_test_helper.hpp"
@@ -37,30 +35,6 @@ std::reverse_iterator<It> mkRev(It it) {
return std::reverse_iterator<It>(it);
}
-
-template <class Iter1, class Iter2>
-bool checkCollectionsEqualVerbose(
- Iter1 start1, Iter1 const end1
- , Iter2 start2, Iter2 const end2
- )
-{
- while (start1 != end1 && start2 != end2) {
- std::cout << "Got start1 = " << *start1 << "\n"
- << "Got start2 = " << *start2 << "\n";
- if (*start1 != *start2) {
- return false;
- }
- ++start1; ++start2;
- }
- if (start1 != end1) {
- std::cout << "Got start1 = " << *start1 << " but expected end1\n";
- }
- if (start2 != end2) {
- std::cout << "Got start2 = " << *start2 << " but expected end2\n";
- }
- return (start1 == end1 && start2 == end2);
-}
-
void checkIteratorConcepts() {
using namespace fs;
using It = path::iterator;
@@ -112,16 +86,15 @@ void checkBeginEndBasic() {
path p("//root_name//first_dir////second_dir");
const path expect[] = {"//root_name", "/", "first_dir", "second_dir"};
assert(checkCollectionsEqual(p.begin(), p.end(), std::begin(expect), std::end(expect)));
- assert(checkCollectionsEqualVerbose(mkRev(p.end()), mkRev(p.begin()),
- mkRev(std::end(expect)),
- mkRev(std::begin(expect))));
+ assert(checkCollectionsEqualBackwards(p.begin(), p.end(), std::begin(expect), std::end(expect)));
+
}
{
path p("////foo/bar/baz///");
const path expect[] = {"/", "foo", "bar", "baz", "."};
assert(checkCollectionsEqual(p.begin(), p.end(), std::begin(expect), std::end(expect)));
- assert(checkCollectionsEqual(mkRev(p.end()), mkRev(p.begin()),
- mkRev(std::end(expect)), mkRev(std::begin(expect))));
+ assert(checkCollectionsEqualBackwards(p.begin(), p.end(), std::begin(expect), std::end(expect)));
+
}
}
Modified: libcxx/trunk/test/std/experimental/filesystem/class.path/path.member/path.native.obs/string_alloc.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/class.path/path.member/path.native.obs/string_alloc.pass.cpp?rev=273078&r1=273077&r2=273078&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/class.path/path.member/path.native.obs/string_alloc.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/class.path/path.member/path.native.obs/string_alloc.pass.cpp Fri Jun 17 23:10:23 2016
@@ -30,7 +30,8 @@
namespace fs = std::experimental::filesystem;
-MultiStringType shortString = MKSTR("abc");
+// the SSO is always triggered for strings of size 2.
+MultiStringType shortString = MKSTR("a");
MultiStringType longString = MKSTR("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ/123456789/abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ");
template <class CharT>
@@ -43,8 +44,6 @@ void doShortStringTest(MultiStringType c
const path p((const char*)MS);
{
DisableAllocationGuard g;
- if (!std::is_same<CharT, char>::value)
- g.release();
Str s = p.string<CharT>();
assert(s == value);
Str s2 = p.string<CharT>(Alloc{});
@@ -55,6 +54,7 @@ void doShortStringTest(MultiStringType c
{
using Traits = std::char_traits<CharT>;
using AStr = std::basic_string<CharT, Traits, MAlloc>;
+ DisableAllocationGuard g;
AStr s = p.string<CharT, Traits, MAlloc>();
assert(s == value);
assert(MAlloc::alloc_count == 0);
@@ -64,6 +64,7 @@ void doShortStringTest(MultiStringType c
{ // Other allocator - provided copy
using Traits = std::char_traits<CharT>;
using AStr = std::basic_string<CharT, Traits, MAlloc>;
+ DisableAllocationGuard g;
MAlloc a;
// don't allow another allocator to be default constructed.
MAlloc::disable_default_constructor = true;
@@ -94,6 +95,7 @@ void doLongStringTest(MultiStringType co
{ // Other allocator - default construct
using Traits = std::char_traits<CharT>;
using AStr = std::basic_string<CharT, Traits, MAlloc>;
+ DisableAllocationGuard g;
AStr s = p.string<CharT, Traits, MAlloc>();
assert(s == value);
assert(MAlloc::alloc_count > 0);
@@ -103,6 +105,7 @@ void doLongStringTest(MultiStringType co
{ // Other allocator - provided copy
using Traits = std::char_traits<CharT>;
using AStr = std::basic_string<CharT, Traits, MAlloc>;
+ DisableAllocationGuard g;
MAlloc a;
// don't allow another allocator to be default constructed.
MAlloc::disable_default_constructor = true;
Modified: libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.space/space.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.space/space.pass.cpp?rev=273078&r1=273077&r2=273078&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.space/space.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.space/space.pass.cpp Fri Jun 17 23:10:23 2016
@@ -88,13 +88,22 @@ TEST_CASE(basic_space_test)
TEST_CHECK(expect.f_bfree > 0);
TEST_CHECK(expect.f_bsize > 0);
TEST_CHECK(expect.f_blocks > 0);
- TEST_CHECK(expect.f_frsize > 0);
+ TEST_REQUIRE(expect.f_frsize > 0);
+ auto do_mult = [&](std::uintmax_t val) {
+ std::uintmax_t fsize = expect.f_frsize;
+ std::uintmax_t new_val = val * fsize;
+ TEST_CHECK(new_val / fsize == val); // Test for overflow
+ return new_val;
+ };
const std::uintmax_t bad_value = static_cast<std::uintmax_t>(-1);
- const std::uintmax_t expect_cap = expect.f_blocks * expect.f_frsize;
+ const std::uintmax_t expect_capacity = do_mult(expect.f_blocks);
+ const std::uintmax_t expect_free = do_mult(expect.f_bfree);
+ const std::uintmax_t expect_avail = do_mult(expect.f_bavail);
+
// Other processes running on the operating system may have changed
// the amount of space available. Check that these are within tolerances.
// Currently 5% of capacity
- const std::uintmax_t delta = expect_cap / 20;
+ const std::uintmax_t delta = expect_capacity / 20;
const path cases[] = {
StaticEnv::File,
StaticEnv::Dir,
@@ -107,11 +116,11 @@ TEST_CASE(basic_space_test)
space_info info = space(p, ec);
TEST_CHECK(!ec);
TEST_CHECK(info.capacity != bad_value);
- TEST_CHECK((expect.f_blocks * expect.f_frsize) == info.capacity);
+ TEST_CHECK(expect_capacity == info.capacity);
TEST_CHECK(info.free != bad_value);
- TEST_CHECK(EqualDelta((expect.f_bfree * expect.f_frsize), info.free, delta));
+ TEST_CHECK(EqualDelta(expect_free, info.free, delta));
TEST_CHECK(info.available != bad_value);
- TEST_CHECK(EqualDelta((expect.f_bavail * expect.f_frsize), info.available, delta));
+ TEST_CHECK(EqualDelta(expect_avail, info.available, delta));
}
}
Modified: libcxx/trunk/test/support/filesystem_test_helper.hpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/support/filesystem_test_helper.hpp?rev=273078&r1=273077&r2=273078&view=diff
==============================================================================
--- libcxx/trunk/test/support/filesystem_test_helper.hpp (original)
+++ libcxx/trunk/test/support/filesystem_test_helper.hpp Fri Jun 17 23:10:23 2016
@@ -361,6 +361,22 @@ bool checkCollectionsEqual(
return (start1 == end1 && start2 == end2);
}
+
+template <class Iter1, class Iter2>
+bool checkCollectionsEqualBackwards(
+ Iter1 const start1, Iter1 end1
+ , Iter2 const start2, Iter2 end2
+ )
+{
+ while (start1 != end1 && start2 != end2) {
+ --end1; --end2;
+ if (*end1 != *end2) {
+ return false;
+ }
+ }
+ return (start1 == end1 && start2 == end2);
+}
+
// We often need to test that the error_code was cleared if no error occurs
// this function returns a error_code which is set to an error that will
// never be returned by the filesystem functions.
More information about the cfe-commits
mailing list