From f1d7a10e06f0f26d02e86c340d93a482b2c50e17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Thu, 25 Mar 2021 20:02:47 +0100 Subject: [PATCH] Remove StringRef::isNullTerminated and StringRef::c_str Both of them were fundamentally unsafe to use and shouldn't be used at all. --- src/catch2/internal/catch_stringref.cpp | 6 -- src/catch2/internal/catch_stringref.hpp | 8 --- .../Baselines/compact.sw.approved.txt | 17 +---- .../Baselines/console.std.approved.txt | 2 +- .../Baselines/console.sw.approved.txt | 54 ++------------- .../SelfTest/Baselines/junit.sw.approved.txt | 2 +- tests/SelfTest/Baselines/tap.sw.approved.txt | 30 ++------ tests/SelfTest/Baselines/xml.sw.approved.txt | 68 +++---------------- .../IntrospectiveTests/String.tests.cpp | 17 +---- 9 files changed, 26 insertions(+), 178 deletions(-) diff --git a/src/catch2/internal/catch_stringref.cpp b/src/catch2/internal/catch_stringref.cpp index a318857c..36a6d246 100644 --- a/src/catch2/internal/catch_stringref.cpp +++ b/src/catch2/internal/catch_stringref.cpp @@ -5,7 +5,6 @@ // https://www.boost.org/LICENSE_1_0.txt) // SPDX-License-Identifier: BSL-1.0 -#include #include #include @@ -18,11 +17,6 @@ namespace Catch { : StringRef( rawChars, static_cast(std::strlen(rawChars) ) ) {} - auto StringRef::c_str() const -> char const* { - CATCH_ENFORCE(isNullTerminated(), "Called StringRef::c_str() on a non-null-terminated instance"); - return m_start; - } - auto StringRef::operator == ( StringRef const& other ) const noexcept -> bool { return m_size == other.m_size && (std::memcmp( m_start, other.m_start, m_size ) == 0); diff --git a/src/catch2/internal/catch_stringref.hpp b/src/catch2/internal/catch_stringref.hpp index 33b35ef6..fed35b16 100644 --- a/src/catch2/internal/catch_stringref.hpp +++ b/src/catch2/internal/catch_stringref.hpp @@ -69,10 +69,6 @@ namespace Catch { return m_size; } - // Returns the current start pointer. If the StringRef is not - // null-terminated, throws std::domain_exception - auto c_str() const -> char const*; - public: // substrings and searches // Returns a substring of [start, start + length). // If start + length > size(), then the substring is [start, start + size()). @@ -91,10 +87,6 @@ namespace Catch { return m_start; } - constexpr auto isNullTerminated() const noexcept -> bool { - return m_start[m_size] == '\0'; - } - public: // iterators constexpr const_iterator begin() const { return m_start; } constexpr const_iterator end() const { return m_start + m_size; } diff --git a/tests/SelfTest/Baselines/compact.sw.approved.txt b/tests/SelfTest/Baselines/compact.sw.approved.txt index 3b19a962..e757222d 100644 --- a/tests/SelfTest/Baselines/compact.sw.approved.txt +++ b/tests/SelfTest/Baselines/compact.sw.approved.txt @@ -1481,29 +1481,23 @@ Matchers.tests.cpp:: passed: testStringForMatching(), EndsWith("sub Matchers.tests.cpp:: passed: testStringForMatching(), EndsWith(" SuBsTrInG", Catch::CaseSensitive::No) for: "this string contains 'abc' as a substring" ends with: " substring" (case insensitive) String.tests.cpp:: passed: empty.empty() for: true String.tests.cpp:: passed: empty.size() == 0 for: 0 == 0 -String.tests.cpp:: passed: empty.isNullTerminated() for: true -String.tests.cpp:: passed: std::strcmp( empty.c_str(), "" ) == 0 for: 0 == 0 +String.tests.cpp:: passed: std::strcmp( empty.data(), "" ) == 0 for: 0 == 0 String.tests.cpp:: passed: s.empty() == false for: false == false String.tests.cpp:: passed: s.size() == 5 for: 5 == 5 -String.tests.cpp:: passed: s.isNullTerminated() for: true String.tests.cpp:: passed: std::strcmp( rawChars, "hello" ) == 0 for: 0 == 0 -String.tests.cpp:: passed: s.c_str() -String.tests.cpp:: passed: s.c_str() == rawChars for: "hello" == "hello" String.tests.cpp:: passed: s.data() == rawChars for: "hello" == "hello" String.tests.cpp:: passed: original == "original" -String.tests.cpp:: passed: !(original.isNullTerminated()) for: !false -String.tests.cpp:: passed: original.c_str() String.tests.cpp:: passed: original.data() String.tests.cpp:: passed: ss.empty() == false for: false == false String.tests.cpp:: passed: ss.size() == 5 for: 5 == 5 String.tests.cpp:: passed: std::strncmp( ss.data(), "hello", 5 ) == 0 for: 0 == 0 String.tests.cpp:: passed: ss == "hello" for: hello == "hello" String.tests.cpp:: passed: ss.size() == 6 for: 6 == 6 -String.tests.cpp:: passed: std::strcmp( ss.c_str(), "world!" ) == 0 for: 0 == 0 +String.tests.cpp:: passed: std::strcmp( ss.data(), "world!" ) == 0 for: 0 == 0 String.tests.cpp:: passed: s.data() == s2.data() for: "hello world!" == "hello world!" String.tests.cpp:: passed: s.data() == ss.data() for: "hello world!" == "hello world!" String.tests.cpp:: passed: s.substr(s.size() + 1, 123).empty() for: true -String.tests.cpp:: passed: std::strcmp(ss.c_str(), "world!") == 0 for: 0 == 0 +String.tests.cpp:: passed: std::strcmp(ss.data(), "world!") == 0 for: 0 == 0 String.tests.cpp:: passed: s.substr(1'000'000, 1).empty() for: true String.tests.cpp:: passed: reinterpret_cast(buffer1) != reinterpret_cast(buffer2) for: "Hello" != "Hello" String.tests.cpp:: passed: left == right for: Hello == Hello @@ -1525,7 +1519,6 @@ String.tests.cpp:: passed: together == "abrakadabra" for: "abrakada String.tests.cpp:: passed: with 1 message: 'empty.size() == 0' String.tests.cpp:: passed: with 1 message: 'empty.begin() == empty.end()' String.tests.cpp:: passed: with 1 message: 'stringref.size() == 3' -String.tests.cpp:: passed: with 1 message: 'stringref.isNullTerminated()' String.tests.cpp:: passed: with 1 message: 'stringref.data() == abc' String.tests.cpp:: passed: with 1 message: 'stringref.begin() == abc' String.tests.cpp:: passed: with 1 message: 'stringref.begin() != stringref.end()' @@ -1535,14 +1528,10 @@ String.tests.cpp:: passed: with 1 message: 'stringref[1] == 'b'' String.tests.cpp:: passed: with 1 message: 'shortened.size() == 2' String.tests.cpp:: passed: with 1 message: 'shortened.data() == abc' String.tests.cpp:: passed: with 1 message: 'shortened.begin() != shortened.end()' -String.tests.cpp:: passed: with 1 message: '!(shortened.isNullTerminated())' -String.tests.cpp:: passed: with 1 message: '!(shortened.substr(1, 3).isNullTerminated())' String.tests.cpp:: passed: with 1 message: '!(sr1.empty())' String.tests.cpp:: passed: with 1 message: 'sr1.size() == 3' -String.tests.cpp:: passed: with 1 message: 'sr1.isNullTerminated()' String.tests.cpp:: passed: with 1 message: 'sr2.empty()' String.tests.cpp:: passed: with 1 message: 'sr2.size() == 0' -String.tests.cpp:: passed: with 1 message: 'sr2.isNullTerminated()' ToStringChrono.tests.cpp:: passed: minute == seconds for: 1 m == 60 s ToStringChrono.tests.cpp:: passed: hour != seconds for: 1 h != 60 s ToStringChrono.tests.cpp:: passed: micro != milli for: 1 us != 1 ms diff --git a/tests/SelfTest/Baselines/console.std.approved.txt b/tests/SelfTest/Baselines/console.std.approved.txt index d781a60e..f91f4b80 100644 --- a/tests/SelfTest/Baselines/console.std.approved.txt +++ b/tests/SelfTest/Baselines/console.std.approved.txt @@ -1381,5 +1381,5 @@ due to unexpected exception with message: =============================================================================== test cases: 356 | 282 passed | 70 failed | 4 failed as expected -assertions: 2088 | 1936 passed | 131 failed | 21 failed as expected +assertions: 2077 | 1925 passed | 131 failed | 21 failed as expected diff --git a/tests/SelfTest/Baselines/console.sw.approved.txt b/tests/SelfTest/Baselines/console.sw.approved.txt index 7066d593..5f77647a 100644 --- a/tests/SelfTest/Baselines/console.sw.approved.txt +++ b/tests/SelfTest/Baselines/console.sw.approved.txt @@ -10494,12 +10494,7 @@ with expansion: 0 == 0 String.tests.cpp:: PASSED: - REQUIRE( empty.isNullTerminated() ) -with expansion: - true - -String.tests.cpp:: PASSED: - REQUIRE( std::strcmp( empty.c_str(), "" ) == 0 ) + REQUIRE( std::strcmp( empty.data(), "" ) == 0 ) with expansion: 0 == 0 @@ -10520,24 +10515,11 @@ String.tests.cpp:: PASSED: with expansion: 5 == 5 -String.tests.cpp:: PASSED: - REQUIRE( s.isNullTerminated() ) -with expansion: - true - String.tests.cpp:: PASSED: REQUIRE( std::strcmp( rawChars, "hello" ) == 0 ) with expansion: 0 == 0 -String.tests.cpp:: PASSED: - REQUIRE_NOTHROW( s.c_str() ) - -String.tests.cpp:: PASSED: - REQUIRE( s.c_str() == rawChars ) -with expansion: - "hello" == "hello" - String.tests.cpp:: PASSED: REQUIRE( s.data() == rawChars ) with expansion: @@ -10553,14 +10535,6 @@ String.tests.cpp: String.tests.cpp:: PASSED: REQUIRE( original == "original" ) -String.tests.cpp:: PASSED: - REQUIRE_FALSE( original.isNullTerminated() ) -with expansion: - !false - -String.tests.cpp:: PASSED: - REQUIRE_THROWS( original.c_str() ) - String.tests.cpp:: PASSED: REQUIRE_NOTHROW( original.data() ) @@ -10606,7 +10580,7 @@ with expansion: 6 == 6 String.tests.cpp:: PASSED: - REQUIRE( std::strcmp( ss.c_str(), "world!" ) == 0 ) + REQUIRE( std::strcmp( ss.data(), "world!" ) == 0 ) with expansion: 0 == 0 @@ -10658,7 +10632,7 @@ String.tests.cpp: ............................................................................... String.tests.cpp:: PASSED: - REQUIRE( std::strcmp(ss.c_str(), "world!") == 0 ) + REQUIRE( std::strcmp(ss.data(), "world!") == 0 ) with expansion: 0 == 0 @@ -10832,10 +10806,6 @@ String.tests.cpp:: PASSED: with message: stringref.size() == 3 -String.tests.cpp:: PASSED: -with message: - stringref.isNullTerminated() - String.tests.cpp:: PASSED: with message: stringref.data() == abc @@ -10872,14 +10842,6 @@ String.tests.cpp:: PASSED: with message: shortened.begin() != shortened.end() -String.tests.cpp:: PASSED: -with message: - !(shortened.isNullTerminated()) - -String.tests.cpp:: PASSED: -with message: - !(shortened.substr(1, 3).isNullTerminated()) - ------------------------------------------------------------------------------- StringRef at compilation time UDL construction @@ -10895,10 +10857,6 @@ String.tests.cpp:: PASSED: with message: sr1.size() == 3 -String.tests.cpp:: PASSED: -with message: - sr1.isNullTerminated() - String.tests.cpp:: PASSED: with message: sr2.empty() @@ -10907,10 +10865,6 @@ String.tests.cpp:: PASSED: with message: sr2.size() == 0 -String.tests.cpp:: PASSED: -with message: - sr2.isNullTerminated() - ------------------------------------------------------------------------------- Stringifying std::chrono::duration helpers ------------------------------------------------------------------------------- @@ -16769,5 +16723,5 @@ Misc.tests.cpp:: PASSED: =============================================================================== test cases: 356 | 266 passed | 86 failed | 4 failed as expected -assertions: 2105 | 1936 passed | 148 failed | 21 failed as expected +assertions: 2094 | 1925 passed | 148 failed | 21 failed as expected diff --git a/tests/SelfTest/Baselines/junit.sw.approved.txt b/tests/SelfTest/Baselines/junit.sw.approved.txt index 5206ed2d..12a1456f 100644 --- a/tests/SelfTest/Baselines/junit.sw.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.approved.txt @@ -1,7 +1,7 @@ - + diff --git a/tests/SelfTest/Baselines/tap.sw.approved.txt b/tests/SelfTest/Baselines/tap.sw.approved.txt index edf6d1ed..8bb648f5 100644 --- a/tests/SelfTest/Baselines/tap.sw.approved.txt +++ b/tests/SelfTest/Baselines/tap.sw.approved.txt @@ -2646,30 +2646,18 @@ ok {test-number} - empty.empty() for: true # StringRef ok {test-number} - empty.size() == 0 for: 0 == 0 # StringRef -ok {test-number} - empty.isNullTerminated() for: true -# StringRef -ok {test-number} - std::strcmp( empty.c_str(), "" ) == 0 for: 0 == 0 +ok {test-number} - std::strcmp( empty.data(), "" ) == 0 for: 0 == 0 # StringRef ok {test-number} - s.empty() == false for: false == false # StringRef ok {test-number} - s.size() == 5 for: 5 == 5 # StringRef -ok {test-number} - s.isNullTerminated() for: true -# StringRef ok {test-number} - std::strcmp( rawChars, "hello" ) == 0 for: 0 == 0 # StringRef -ok {test-number} - s.c_str() -# StringRef -ok {test-number} - s.c_str() == rawChars for: "hello" == "hello" -# StringRef ok {test-number} - s.data() == rawChars for: "hello" == "hello" # StringRef ok {test-number} - original == "original" # StringRef -ok {test-number} - !(original.isNullTerminated()) for: !false -# StringRef -ok {test-number} - original.c_str() -# StringRef ok {test-number} - original.data() # StringRef ok {test-number} - ss.empty() == false for: false == false @@ -2682,7 +2670,7 @@ ok {test-number} - ss == "hello" for: hello == "hello" # StringRef ok {test-number} - ss.size() == 6 for: 6 == 6 # StringRef -ok {test-number} - std::strcmp( ss.c_str(), "world!" ) == 0 for: 0 == 0 +ok {test-number} - std::strcmp( ss.data(), "world!" ) == 0 for: 0 == 0 # StringRef ok {test-number} - s.data() == s2.data() for: "hello world!" == "hello world!" # StringRef @@ -2690,7 +2678,7 @@ ok {test-number} - s.data() == ss.data() for: "hello world!" == "hello world!" # StringRef ok {test-number} - s.substr(s.size() + 1, 123).empty() for: true # StringRef -ok {test-number} - std::strcmp(ss.c_str(), "world!") == 0 for: 0 == 0 +ok {test-number} - std::strcmp(ss.data(), "world!") == 0 for: 0 == 0 # StringRef ok {test-number} - s.substr(1'000'000, 1).empty() for: true # StringRef @@ -2730,8 +2718,6 @@ ok {test-number} - with 1 message: 'empty.begin() == empty.end()' # StringRef at compilation time ok {test-number} - with 1 message: 'stringref.size() == 3' # StringRef at compilation time -ok {test-number} - with 1 message: 'stringref.isNullTerminated()' -# StringRef at compilation time ok {test-number} - with 1 message: 'stringref.data() == abc' # StringRef at compilation time ok {test-number} - with 1 message: 'stringref.begin() == abc' @@ -2750,21 +2736,13 @@ ok {test-number} - with 1 message: 'shortened.data() == abc' # StringRef at compilation time ok {test-number} - with 1 message: 'shortened.begin() != shortened.end()' # StringRef at compilation time -ok {test-number} - with 1 message: '!(shortened.isNullTerminated())' -# StringRef at compilation time -ok {test-number} - with 1 message: '!(shortened.substr(1, 3).isNullTerminated())' -# StringRef at compilation time ok {test-number} - with 1 message: '!(sr1.empty())' # StringRef at compilation time ok {test-number} - with 1 message: 'sr1.size() == 3' # StringRef at compilation time -ok {test-number} - with 1 message: 'sr1.isNullTerminated()' -# StringRef at compilation time ok {test-number} - with 1 message: 'sr2.empty()' # StringRef at compilation time ok {test-number} - with 1 message: 'sr2.size() == 0' -# StringRef at compilation time -ok {test-number} - with 1 message: 'sr2.isNullTerminated()' # Stringifying std::chrono::duration helpers ok {test-number} - minute == seconds for: 1 m == 60 s # Stringifying std::chrono::duration helpers @@ -4202,5 +4180,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0 ok {test-number} - # xmlentitycheck ok {test-number} - -1..2105 +1..2094 diff --git a/tests/SelfTest/Baselines/xml.sw.approved.txt b/tests/SelfTest/Baselines/xml.sw.approved.txt index ceaa32bc..a624d5bd 100644 --- a/tests/SelfTest/Baselines/xml.sw.approved.txt +++ b/tests/SelfTest/Baselines/xml.sw.approved.txt @@ -12536,21 +12536,13 @@ Message from section two - empty.isNullTerminated() - - - true - - - - - std::strcmp( empty.c_str(), "" ) == 0 + std::strcmp( empty.data(), "" ) == 0 0 == 0 - +
@@ -12569,14 +12561,6 @@ Message from section two 5 == 5 - - - s.isNullTerminated() - - - true - - std::strcmp( rawChars, "hello" ) == 0 @@ -12585,22 +12569,6 @@ Message from section two 0 == 0 - - - s.c_str() - - - s.c_str() - - - - - s.c_str() == rawChars - - - "hello" == "hello" - - s.data() == rawChars @@ -12609,7 +12577,7 @@ Message from section two "hello" == "hello" - +
@@ -12620,22 +12588,6 @@ Message from section two original == "original" - - - !(original.isNullTerminated()) - - - !false - - - - - original.c_str() - - - original.c_str() - - original.data() @@ -12644,7 +12596,7 @@ Message from section two original.data() - +
@@ -12696,7 +12648,7 @@ Message from section two - std::strcmp( ss.c_str(), "world!" ) == 0 + std::strcmp( ss.data(), "world!" ) == 0 0 == 0 @@ -12752,7 +12704,7 @@ Message from section two
- std::strcmp(ss.c_str(), "world!") == 0 + std::strcmp(ss.data(), "world!") == 0 0 == 0 @@ -12941,10 +12893,10 @@ Message from section two
- +
- +
@@ -19715,9 +19667,9 @@ loose text artifact
- + - + diff --git a/tests/SelfTest/IntrospectiveTests/String.tests.cpp b/tests/SelfTest/IntrospectiveTests/String.tests.cpp index dbc8190b..4b9cc0e3 100644 --- a/tests/SelfTest/IntrospectiveTests/String.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/String.tests.cpp @@ -10,29 +10,23 @@ TEST_CASE( "StringRef", "[Strings][StringRef]" ) { StringRef empty; REQUIRE( empty.empty() ); REQUIRE( empty.size() == 0 ); - REQUIRE( empty.isNullTerminated() ); - REQUIRE( std::strcmp( empty.c_str(), "" ) == 0 ); + REQUIRE( std::strcmp( empty.data(), "" ) == 0 ); } SECTION( "From string literal" ) { StringRef s = "hello"; REQUIRE( s.empty() == false ); REQUIRE( s.size() == 5 ); - REQUIRE( s.isNullTerminated() ); auto rawChars = s.data(); REQUIRE( std::strcmp( rawChars, "hello" ) == 0 ); - REQUIRE_NOTHROW(s.c_str()); - REQUIRE(s.c_str() == rawChars); REQUIRE(s.data() == rawChars); } SECTION( "From sub-string" ) { StringRef original = StringRef( "original string" ).substr(0, 8); REQUIRE( original == "original" ); - REQUIRE_FALSE(original.isNullTerminated()); - REQUIRE_THROWS(original.c_str()); REQUIRE_NOTHROW(original.data()); } @@ -51,7 +45,7 @@ TEST_CASE( "StringRef", "[Strings][StringRef]" ) { SECTION( "non-zero-based substring") { ss = s.substr( 6, 6 ); REQUIRE( ss.size() == 6 ); - REQUIRE( std::strcmp( ss.c_str(), "world!" ) == 0 ); + REQUIRE( std::strcmp( ss.data(), "world!" ) == 0 ); } SECTION( "Pointer values of full refs should match" ) { @@ -69,7 +63,7 @@ TEST_CASE( "StringRef", "[Strings][StringRef]" ) { SECTION("Substring off the end are trimmed") { ss = s.substr(6, 123); - REQUIRE(std::strcmp(ss.c_str(), "world!") == 0); + REQUIRE(std::strcmp(ss.data(), "world!") == 0); } SECTION("substring start after the end is empty") { REQUIRE(s.substr(1'000'000, 1).empty()); @@ -147,7 +141,6 @@ TEST_CASE("StringRef at compilation time", "[Strings][StringRef][constexpr]") { constexpr StringRef stringref(abc, 3); STATIC_REQUIRE(stringref.size() == 3); - STATIC_REQUIRE(stringref.isNullTerminated()); STATIC_REQUIRE(stringref.data() == abc); STATIC_REQUIRE(stringref.begin() == abc); STATIC_REQUIRE(stringref.begin() != stringref.end()); @@ -160,19 +153,15 @@ TEST_CASE("StringRef at compilation time", "[Strings][StringRef][constexpr]") { STATIC_REQUIRE(shortened.size() == 2); STATIC_REQUIRE(shortened.data() == abc); STATIC_REQUIRE(shortened.begin() != shortened.end()); - STATIC_REQUIRE_FALSE(shortened.isNullTerminated()); - STATIC_REQUIRE_FALSE(shortened.substr(1, 3).isNullTerminated()); } SECTION("UDL construction") { constexpr auto sr1 = "abc"_catch_sr; STATIC_REQUIRE_FALSE(sr1.empty()); STATIC_REQUIRE(sr1.size() == 3); - STATIC_REQUIRE(sr1.isNullTerminated()); using Catch::operator"" _sr; constexpr auto sr2 = ""_sr; STATIC_REQUIRE(sr2.empty()); STATIC_REQUIRE(sr2.size() == 0); - STATIC_REQUIRE(sr2.isNullTerminated()); } }