From 2017595dfa6cb6a3629a493491b49f263b7c06ee Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Mon, 17 Dec 2018 22:43:31 +0000 Subject: [PATCH] PR libstdc++/71044 fix off-by-one errors introduced recently The recent changes to append/concat directly from strings (without constructing paths) introduced regressions where one of the components could be omitted from the iteration sequence in the result. PR libstdc++/71044 * src/filesystem/std-path.cc (path::_M_append): Fix off-by-one error that caused a component to be lost from the iteration sequence. (path::_M_concat): Likewise. * testsuite/27_io/filesystem/path/append/source.cc: Test appending long strings. * testsuite/27_io/filesystem/path/concat/strings.cc: Test concatenating long strings. * testsuite/27_io/filesystem/path/construct/string_view.cc: Test construction from long string. From-SVN: r267222 --- libstdc++-v3/ChangeLog | 13 +++++++ libstdc++-v3/src/filesystem/std-path.cc | 7 ++-- .../27_io/filesystem/path/append/source.cc | 17 ++++++++ .../27_io/filesystem/path/concat/strings.cc | 39 +++++++++++++++++-- .../filesystem/path/construct/string_view.cc | 24 ++++++++++++ 5 files changed, 93 insertions(+), 7 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index a4f6507c2bb..d9dc973a440 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,16 @@ +2018-12-17 Jonathan Wakely + + PR libstdc++/71044 + * src/filesystem/std-path.cc (path::_M_append): Fix off-by-one error + that caused a component to be lost from the iteration sequence. + (path::_M_concat): Likewise. + * testsuite/27_io/filesystem/path/append/source.cc: Test appending + long strings. + * testsuite/27_io/filesystem/path/concat/strings.cc: Test + concatenating long strings. + * testsuite/27_io/filesystem/path/construct/string_view.cc: Test + construction from long string. + 2018-12-13 Jonathan Wakely * src/filesystem/std-path.cc (SLASHSLASH_IS_ROOT_NAME): New macro to diff --git a/libstdc++-v3/src/filesystem/std-path.cc b/libstdc++-v3/src/filesystem/std-path.cc index d2520492c03..b5ddbdad149 100644 --- a/libstdc++-v3/src/filesystem/std-path.cc +++ b/libstdc++-v3/src/filesystem/std-path.cc @@ -781,10 +781,11 @@ path::_M_append(basic_string_view s) ::new(output++) _Cmpt(c.str, c.type, parser.offset(c)); ++_M_cmpts._M_impl->_M_size; } - for (auto c = parser.next(); c.valid(); c = parser.next()) + while (cmpt.valid()) { - ::new(output++) _Cmpt(c.str, c.type, parser.offset(c)); + ::new(output++) _Cmpt(cmpt.str, cmpt.type, parser.offset(cmpt)); ++_M_cmpts._M_impl->_M_size; + cmpt = parser.next(); } if (s.back() == '/') @@ -1139,7 +1140,7 @@ path::_M_concat(basic_string_view s) } #endif } - else if (orig_filenamelen == 0) + else if (orig_filenamelen == 0 && extra.empty()) { // Replace empty filename at end of original path. std::prev(output)->_M_pathname = it->str; diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/append/source.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/append/source.cc index e440ca921c7..21ae6be3d97 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/path/append/source.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/path/append/source.cc @@ -137,6 +137,22 @@ test05() s = second->native(); p3 /= s; VERIFY( p3.string() == "0/123456789/a/123456789" ); + } + +void +test06() +{ + const std::string s0 = "a/b/c"; + path p = s0; + std::string s; + for (int i = 0; i < 10; ++i) + s += "0/1/2/3/4/5/6/7/8/9/"; + // append a long string with many components + test(p, s.c_str()); + + // Same again but with a trailing slash on the left operand: + path p2 = s0 + '/'; + test(p2, s.c_str()); } int @@ -147,4 +163,5 @@ main() test03(); test04(); test05(); + test06(); } diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/concat/strings.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/concat/strings.cc index eea9b6dc69b..316f7c3d7bd 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/path/concat/strings.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/path/concat/strings.cc @@ -24,8 +24,10 @@ #include #include #include +#include using std::filesystem::path; +using __gnu_test::compare_paths; void test01() @@ -60,7 +62,7 @@ test01() void test02() { - std::basic_string_view s; + std::basic_string_view s, expected; path p = "0/1/2/3/4/5/6"; // The string_view aliases the path's internal string: @@ -68,25 +70,54 @@ test02() // Append that string_view, which must work correctly even though the // internal string will be reallocated during the operation: p += s; - VERIFY( p.string() == "0/1/2/3/4/5/60/1/2/3/4/5/6" ); + compare_paths(p, "0/1/2/3/4/5/60/1/2/3/4/5/6"); // Same again with a trailing slash: path p2 = "0/1/2/3/4/5/"; s = p2.native(); p2 += s; - VERIFY( p2.string() == "0/1/2/3/4/5/0/1/2/3/4/5/" ); + compare_paths(p2, "0/1/2/3/4/5/0/1/2/3/4/5/"); // And aliasing one of the components of the path: path p3 = "0/123456789"; path::iterator second = std::next(p3.begin()); s = second->native(); p3 += s; - VERIFY( p3.string() == "0/123456789123456789" ); + compare_paths(p3, "0/123456789123456789" ); } +void +test03() +{ + const std::string s0 = "a/b/c"; + path p = s0; + std::string s; + for (int i = 0; i < 10; ++i) + s += "0/1/2/3/4/5/6/7/8/9/"; + // concat a long string with many components: + p += s; + compare_paths(p, path(s0+s)); + + // Same again but with a trailing slash on the left operand: + path p2 = s0 + '/'; + p2 += s; + compare_paths(p2, path(s0+'/'+s)); + + // And again but with a leading slash on the right operand: + path p3 = s0; + s.insert(0, 1, '/'); + p3 += s; + compare_paths(p2, path(s0+s)); + + // And again but with a slash on both operands: + path p4 = s0 + '/'; + p4 += s; + compare_paths(p4, path(s0+'/'+s)); +} int main() { test01(); test02(); + test03(); } diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/construct/string_view.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/string_view.cc index 0339df73b30..a6130d389bf 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/path/construct/string_view.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/string_view.cc @@ -25,6 +25,7 @@ #include #include #include +#include using std::filesystem::path; using __gnu_test::compare_paths; @@ -49,8 +50,31 @@ test01() } } +void +test02() +{ + std::string s; + for (int i = 0; i < 10; ++i) + s += "0/1/2/3/4/5/6/7/8/9/"; + // Construct a path with a large number of components: + path p = s; + auto iter = p.begin(); + for (int i = 0; i < 100; ++i) + { + char c = '0' + i % 10; + std::string_view sv(&c, 1); + VERIFY( iter != p.end() ); + compare_paths( *iter, sv ); + ++iter; + } + VERIFY( iter != p.end() ); + VERIFY( iter->native().empty() ); + VERIFY( ++iter == p.end() ); +} + int main() { test01(); + test02(); }