From 96355da34edf51cc813400115984050bd7c50630 Mon Sep 17 00:00:00 2001 From: Phil Nash Date: Tue, 21 May 2019 00:04:19 +0100 Subject: [PATCH] StringRef no longer repoints m_start to m_data after c_str() on a substring. This fixes an issue where a self-assignment of a StringRef copy would point into internally (and now dangling) data. (now self-assignment check is no longer needed) --- include/internal/catch_stringref.cpp | 9 +++++---- include/internal/catch_stringref.h | 4 ++-- projects/SelfTest/IntrospectiveTests/String.tests.cpp | 8 +++----- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/include/internal/catch_stringref.cpp b/include/internal/catch_stringref.cpp index 9e9095e4..90a320c9 100644 --- a/include/internal/catch_stringref.cpp +++ b/include/internal/catch_stringref.cpp @@ -39,9 +39,11 @@ namespace Catch { } auto StringRef::c_str() const -> char const* { - if( isSubstring() ) - const_cast( this )->takeOwnership(); - return m_start; + if( !isSubstring() ) + return m_start; + + const_cast( this )->takeOwnership(); + return m_data; } auto StringRef::currentData() const noexcept -> char const* { return m_start; @@ -59,7 +61,6 @@ namespace Catch { m_data = new char[m_size+1]; memcpy( m_data, m_start, m_size ); m_data[m_size] = '\0'; - m_start = m_data; } } auto StringRef::substr( size_type start, size_type size ) const noexcept -> StringRef { diff --git a/include/internal/catch_stringref.h b/include/internal/catch_stringref.h index 4e41bd47..70f68476 100644 --- a/include/internal/catch_stringref.h +++ b/include/internal/catch_stringref.h @@ -71,12 +71,12 @@ namespace Catch { } auto operator = ( StringRef const &other ) noexcept -> StringRef& { - if( this != &other ) { +// if( this != &other ) { delete[] m_data; m_data = nullptr; m_start = other.m_start; m_size = other.m_size; - } + // } return *this; } diff --git a/projects/SelfTest/IntrospectiveTests/String.tests.cpp b/projects/SelfTest/IntrospectiveTests/String.tests.cpp index 8bf2ffb3..876d4a09 100644 --- a/projects/SelfTest/IntrospectiveTests/String.tests.cpp +++ b/projects/SelfTest/IntrospectiveTests/String.tests.cpp @@ -65,7 +65,6 @@ TEST_CASE( "StringRef", "[Strings][StringRef]" ) { original.c_str(); // Forces it to take ownership - REQUIRE( isSubstring( original ) == false ); REQUIRE( isOwned( original ) ); } @@ -88,14 +87,13 @@ TEST_CASE( "StringRef", "[Strings][StringRef]" ) { REQUIRE( rawChars == s.currentData() ); // same pointer value REQUIRE( ss.c_str() != rawChars ); - REQUIRE( isSubstring( ss ) == false ); REQUIRE( isOwned( ss ) ); - REQUIRE( ss.currentData() != s.currentData() ); // different pointer value - SECTION( "Self-assignment after substring" ) { ss = *&ss; // the *& are there to suppress warnings (see: "Improvements to Clang's diagnostics" in https://rev.ng/gitlab/revng-bar-2019/clang/raw/master/docs/ReleaseNotes.rst) - REQUIRE(isOwned(ss) == true); + REQUIRE( isOwned(ss) == false ); + REQUIRE( ss == "hello" ); + REQUIRE( rawChars == ss.currentData() ); // same pointer value } }