From ab3062113857871ba1344158c20cbb09b094be4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Fri, 6 Apr 2018 11:39:40 +0200 Subject: [PATCH] Fix stringifying static array of unsigned chars The fix leaves an open question: should we keep treating refs to static array of chars as strings, or should we instead use `strnlen` to check if it is null-terminated within the buffer Fixes #1238 --- include/internal/catch_tostring.h | 13 ++++--- .../Baselines/compact.sw.approved.txt | 2 ++ .../Baselines/console.std.approved.txt | 4 +-- .../Baselines/console.sw.approved.txt | 28 +++++++++++++-- .../Baselines/console.swa4.approved.txt | 28 +++++++++++++-- .../SelfTest/Baselines/junit.sw.approved.txt | 3 +- .../SelfTest/Baselines/xml.sw.approved.txt | 35 +++++++++++++++++-- .../SelfTest/UsageTests/Compilation.tests.cpp | 13 +++++++ 8 files changed, 112 insertions(+), 14 deletions(-) diff --git a/include/internal/catch_tostring.h b/include/internal/catch_tostring.h index 16d52f94..79e1001f 100644 --- a/include/internal/catch_tostring.h +++ b/include/internal/catch_tostring.h @@ -155,6 +155,7 @@ namespace Catch { struct StringMaker { static std::string convert(char * str); }; + #ifdef CATCH_CONFIG_WCHAR template<> struct StringMaker { @@ -166,22 +167,24 @@ namespace Catch { }; #endif + // TBD: Should we use `strnlen` to ensure that we don't go out of the buffer, + // while keeping string semantics? template struct StringMaker { - static std::string convert(const char* str) { + static std::string convert(char const* str) { return ::Catch::Detail::stringify(std::string{ str }); } }; template struct StringMaker { - static std::string convert(const char* str) { - return ::Catch::Detail::stringify(std::string{ str }); + static std::string convert(signed char const* str) { + return ::Catch::Detail::stringify(std::string{ reinterpret_cast(str) }); } }; template struct StringMaker { - static std::string convert(const char* str) { - return ::Catch::Detail::stringify(std::string{ str }); + static std::string convert(unsigned char const* str) { + return ::Catch::Detail::stringify(std::string{ reinterpret_cast(str) }); } }; diff --git a/projects/SelfTest/Baselines/compact.sw.approved.txt b/projects/SelfTest/Baselines/compact.sw.approved.txt index ccc8dbae..dbf34fd4 100644 --- a/projects/SelfTest/Baselines/compact.sw.approved.txt +++ b/projects/SelfTest/Baselines/compact.sw.approved.txt @@ -10,6 +10,8 @@ Compilation.tests.cpp:: passed: t1 > t2 for: {?} > {?} Compilation.tests.cpp:: passed: t1 <= t2 for: {?} <= {?} Compilation.tests.cpp:: passed: t1 >= t2 for: {?} >= {?} Misc.tests.cpp:: passed: +Compilation.tests.cpp:: passed: std::memcmp(uarr, "123", sizeof(uarr)) == 0 for: 0 == 0 with 2 messages: 'uarr := "123"' and 'sarr := "456"' +Compilation.tests.cpp:: passed: std::memcmp(sarr, "456", sizeof(sarr)) == 0 for: 0 == 0 with 2 messages: 'uarr := "123"' and 'sarr := "456"' Exception.tests.cpp:: failed: unexpected exception with message: 'answer := 42' with 1 message: 'expected exception' Exception.tests.cpp:: failed: unexpected exception with message: 'answer := 42'; expression was: thisThrows() with 1 message: 'expected exception' Exception.tests.cpp:: passed: thisThrows() with 1 message: 'answer := 42' diff --git a/projects/SelfTest/Baselines/console.std.approved.txt b/projects/SelfTest/Baselines/console.std.approved.txt index c5b91182..8bef5356 100644 --- a/projects/SelfTest/Baselines/console.std.approved.txt +++ b/projects/SelfTest/Baselines/console.std.approved.txt @@ -1084,6 +1084,6 @@ due to unexpected exception with message: Why would you throw a std::string? =============================================================================== -test cases: 204 | 151 passed | 49 failed | 4 failed as expected -assertions: 1061 | 933 passed | 107 failed | 21 failed as expected +test cases: 205 | 152 passed | 49 failed | 4 failed as expected +assertions: 1063 | 935 passed | 107 failed | 21 failed as expected diff --git a/projects/SelfTest/Baselines/console.sw.approved.txt b/projects/SelfTest/Baselines/console.sw.approved.txt index 814e09af..1e67fd94 100644 --- a/projects/SelfTest/Baselines/console.sw.approved.txt +++ b/projects/SelfTest/Baselines/console.sw.approved.txt @@ -102,6 +102,30 @@ Misc.tests.cpp: Misc.tests.cpp:: PASSED: +------------------------------------------------------------------------------- +#1238 +------------------------------------------------------------------------------- +Compilation.tests.cpp: +............................................................................... + +Compilation.tests.cpp:: +PASSED: + REQUIRE( std::memcmp(uarr, "123", sizeof(uarr)) == 0 ) +with expansion: + 0 == 0 +with messages: + uarr := "123" + sarr := "456" + +Compilation.tests.cpp:: +PASSED: + REQUIRE( std::memcmp(sarr, "456", sizeof(sarr)) == 0 ) +with expansion: + 0 == 0 +with messages: + uarr := "123" + sarr := "456" + ------------------------------------------------------------------------------- #748 - captures with unexpected exceptions outside assertions @@ -8935,6 +8959,6 @@ Misc.tests.cpp:: PASSED: =============================================================================== -test cases: 204 | 138 passed | 62 failed | 4 failed as expected -assertions: 1075 | 933 passed | 121 failed | 21 failed as expected +test cases: 205 | 139 passed | 62 failed | 4 failed as expected +assertions: 1077 | 935 passed | 121 failed | 21 failed as expected diff --git a/projects/SelfTest/Baselines/console.swa4.approved.txt b/projects/SelfTest/Baselines/console.swa4.approved.txt index 9b41d398..b8effce5 100644 --- a/projects/SelfTest/Baselines/console.swa4.approved.txt +++ b/projects/SelfTest/Baselines/console.swa4.approved.txt @@ -102,6 +102,30 @@ Misc.tests.cpp: Misc.tests.cpp:: PASSED: +------------------------------------------------------------------------------- +#1238 +------------------------------------------------------------------------------- +Compilation.tests.cpp: +............................................................................... + +Compilation.tests.cpp:: +PASSED: + REQUIRE( std::memcmp(uarr, "123", sizeof(uarr)) == 0 ) +with expansion: + 0 == 0 +with messages: + uarr := "123" + sarr := "456" + +Compilation.tests.cpp:: +PASSED: + REQUIRE( std::memcmp(sarr, "456", sizeof(sarr)) == 0 ) +with expansion: + 0 == 0 +with messages: + uarr := "123" + sarr := "456" + ------------------------------------------------------------------------------- #748 - captures with unexpected exceptions outside assertions @@ -308,6 +332,6 @@ with expansion: !true =============================================================================== -test cases: 12 | 9 passed | 1 failed | 2 failed as expected -assertions: 35 | 28 passed | 4 failed | 3 failed as expected +test cases: 13 | 10 passed | 1 failed | 2 failed as expected +assertions: 37 | 30 passed | 4 failed | 3 failed as expected diff --git a/projects/SelfTest/Baselines/junit.sw.approved.txt b/projects/SelfTest/Baselines/junit.sw.approved.txt index bc2600e6..63424efe 100644 --- a/projects/SelfTest/Baselines/junit.sw.approved.txt +++ b/projects/SelfTest/Baselines/junit.sw.approved.txt @@ -1,12 +1,13 @@ - + + expected exception diff --git a/projects/SelfTest/Baselines/xml.sw.approved.txt b/projects/SelfTest/Baselines/xml.sw.approved.txt index b6d18960..e2fdb2a4 100644 --- a/projects/SelfTest/Baselines/xml.sw.approved.txt +++ b/projects/SelfTest/Baselines/xml.sw.approved.txt @@ -96,6 +96,37 @@ + + + uarr := "123" + + + sarr := "456" + + + + std::memcmp(uarr, "123", sizeof(uarr)) == 0 + + + 0 == 0 + + + + uarr := "123" + + + sarr := "456" + + + + std::memcmp(sarr, "456", sizeof(sarr)) == 0 + + + 0 == 0 + + + +
@@ -9882,7 +9913,7 @@ loose text artifact
- + - + diff --git a/projects/SelfTest/UsageTests/Compilation.tests.cpp b/projects/SelfTest/UsageTests/Compilation.tests.cpp index 9075743a..3b5f101b 100644 --- a/projects/SelfTest/UsageTests/Compilation.tests.cpp +++ b/projects/SelfTest/UsageTests/Compilation.tests.cpp @@ -7,6 +7,8 @@ #include "catch.hpp" +#include + namespace { namespace CompilationTests { #ifndef COMPILATION_TEST_HELPERS_INCLUDED // Don't compile this more than once per TU @@ -134,4 +136,15 @@ namespace { namespace CompilationTests { REQUIRE(t1 >= t2); } + // unsigned array + TEST_CASE("#1238") { + unsigned char uarr[] = "123"; + CAPTURE(uarr); + signed char sarr[] = "456"; + CAPTURE(sarr); + + REQUIRE(std::memcmp(uarr, "123", sizeof(uarr)) == 0); + REQUIRE(std::memcmp(sarr, "456", sizeof(sarr)) == 0); + } + }} // namespace CompilationTests