From 9634351bd768ed76752e06dceab4e2fcebb72f53 Mon Sep 17 00:00:00 2001 From: Markus Scherer Date: Tue, 28 Feb 2017 06:50:27 +0000 Subject: [PATCH] ICU-12992 test overloads with pointer-wrapper class, add constructor(int null) to match NULL==0 X-SVN-Rev: 39713 --- icu4c/source/common/unicode/unistr.h | 69 +++++++++++++++------ icu4c/source/common/unistr.cpp | 44 +++++++++++++- icu4c/source/test/intltest/ustrtest.cpp | 80 +++++++++++++++++++++++++ icu4c/source/test/intltest/ustrtest.h | 5 ++ 4 files changed, 178 insertions(+), 20 deletions(-) diff --git a/icu4c/source/common/unicode/unistr.h b/icu4c/source/common/unicode/unistr.h index 70b87ac553e..7b325f2df0f 100644 --- a/icu4c/source/common/unicode/unistr.h +++ b/icu4c/source/common/unicode/unistr.h @@ -75,7 +75,7 @@ U_NAMESPACE_BEGIN * and from NULL. * @draft ICU 59 */ -class U_COMMON_API Char16Ptr { +class U_COMMON_API Char16Ptr final { public: /** * Copies the pointer. @@ -87,9 +87,10 @@ public: * @draft ICU 59 */ inline Char16Ptr(uint16_t *p); -#if U_SIZEOF_WCHAR_T==2 +#if U_SIZEOF_WCHAR_T==2 || defined(U_IN_DOXYGEN) /** * Converts the pointer to char16_t *. + * (Only defined if U_SIZEOF_WCHAR_T==2.) * @draft ICU 59 */ inline Char16Ptr(wchar_t *p); @@ -99,13 +100,26 @@ public: * @draft ICU 59 */ inline Char16Ptr(std::nullptr_t p); + /** + * NULL constructor. + * Must only be used for 0 which is usually the value of NULL. + * @draft ICU 59 + */ + Char16Ptr(int null); /** * Pointer access. * @draft ICU 59 */ - inline operator char16_t *(); + inline char16_t *get(); + /** + * Pointer access via type conversion (e.g., static_cast). + * @draft ICU 59 + */ + operator char16_t *() { return get(); } private: + Char16Ptr() = delete; + #ifdef U_ALIASING_BARRIER template char16_t *cast(T *t) { U_ALIASING_BARRIER(t); @@ -131,7 +145,7 @@ Char16Ptr::Char16Ptr(wchar_t *p) : p(cast(p)) {} #endif Char16Ptr::Char16Ptr(std::nullptr_t p) : p(p) {} -Char16Ptr::operator char16_t *() { return p; } +char16_t *Char16Ptr::get() { return p; } #else @@ -142,7 +156,7 @@ Char16Ptr::Char16Ptr(wchar_t *p) { u.wp = p; } #endif Char16Ptr::Char16Ptr(std::nullptr_t p) { u.cp = p; } -Char16Ptr::operator char16_t *() { return u.cp; } +char16_t *Char16Ptr::get() { return u.cp; } #endif @@ -151,7 +165,7 @@ Char16Ptr::operator char16_t *() { return u.cp; } * and from NULL. * @draft ICU 59 */ -class U_COMMON_API ConstChar16Ptr { +class U_COMMON_API ConstChar16Ptr final { public: /** * Copies the pointer. @@ -163,9 +177,10 @@ public: * @draft ICU 59 */ inline ConstChar16Ptr(const uint16_t *p); -#if U_SIZEOF_WCHAR_T==2 +#if U_SIZEOF_WCHAR_T==2 || defined(U_IN_DOXYGEN) /** * Converts the pointer to char16_t *. + * (Only defined if U_SIZEOF_WCHAR_T==2.) * @draft ICU 59 */ inline ConstChar16Ptr(const wchar_t *p); @@ -175,13 +190,26 @@ public: * @draft ICU 59 */ inline ConstChar16Ptr(const std::nullptr_t p); + /** + * NULL constructor. + * Must only be used for 0 which is usually the value of NULL. + * @draft ICU 59 + */ + ConstChar16Ptr(int null); /** * Pointer access. * @draft ICU 59 */ - inline operator const char16_t *() const; + inline const char16_t *get() const; + /** + * Pointer access via type conversion (e.g., static_cast). + * @draft ICU 59 + */ + operator const char16_t *() { return get(); } private: + ConstChar16Ptr() = delete; + #ifdef U_ALIASING_BARRIER template const char16_t *cast(const T *t) { U_ALIASING_BARRIER(t); @@ -207,7 +235,7 @@ ConstChar16Ptr::ConstChar16Ptr(const wchar_t *p) : p(cast(p)) {} #endif ConstChar16Ptr::ConstChar16Ptr(const std::nullptr_t p) : p(p) {} -ConstChar16Ptr::operator const char16_t *() const { return p; } +const char16_t *ConstChar16Ptr::get() const { return p; } #else @@ -218,7 +246,7 @@ ConstChar16Ptr::ConstChar16Ptr(const wchar_t *p) { u.wp = p; } #endif ConstChar16Ptr::ConstChar16Ptr(const std::nullptr_t p) { u.cp = p; } -ConstChar16Ptr::operator const char16_t *() const { return u.cp; } +const char16_t *ConstChar16Ptr::get() const { return u.cp; } #endif @@ -3183,11 +3211,12 @@ public: * @draft ICU 59 */ UNISTR_FROM_STRING_EXPLICIT UnicodeString(const uint16_t *text) : - UnicodeString(static_cast(ConstChar16Ptr(text))) {} + UnicodeString(ConstChar16Ptr(text).get()) {} -#if U_SIZEOF_WCHAR_T==2 +#if U_SIZEOF_WCHAR_T==2 || defined(U_IN_DOXYGEN) /** * wchar_t * constructor. + * (Only defined if U_SIZEOF_WCHAR_T==2.) * Delegates to UnicodeString(const UChar *). * * It is recommended to mark this constructor "explicit" by @@ -3197,7 +3226,7 @@ public: * @draft ICU 59 */ UNISTR_FROM_STRING_EXPLICIT UnicodeString(const wchar_t *text) : - UnicodeString(static_cast(ConstChar16Ptr(text))) {} + UnicodeString(ConstChar16Ptr(text).get()) {} #endif /** @@ -3230,18 +3259,19 @@ public: * @draft ICU 59 */ UnicodeString(const uint16_t *text, int32_t length) : - UnicodeString(static_cast(ConstChar16Ptr(text)), length) {} + UnicodeString(ConstChar16Ptr(text).get(), length) {} -#if U_SIZEOF_WCHAR_T==2 +#if U_SIZEOF_WCHAR_T==2 || defined(U_IN_DOXYGEN) /** * wchar_t * constructor. + * (Only defined if U_SIZEOF_WCHAR_T==2.) * Delegates to UnicodeString(const UChar *, int32_t). * @param text NUL-terminated UTF-16 string * @param length string length * @draft ICU 59 */ UnicodeString(const wchar_t *text, int32_t length) : - UnicodeString(static_cast(ConstChar16Ptr(text)), length) {} + UnicodeString(ConstChar16Ptr(text).get(), length) {} #endif /** @@ -3308,11 +3338,12 @@ public: * @draft ICU 59 */ UnicodeString(uint16_t *buffer, int32_t buffLength, int32_t buffCapacity) : - UnicodeString(static_cast(Char16Ptr(buffer)), buffLength, buffCapacity) {} + UnicodeString(Char16Ptr(buffer).get(), buffLength, buffCapacity) {} -#if U_SIZEOF_WCHAR_T==2 +#if U_SIZEOF_WCHAR_T==2 || defined(U_IN_DOXYGEN) /** * Writable-aliasing wchar_t * constructor. + * (Only defined if U_SIZEOF_WCHAR_T==2.) * Delegates to UnicodeString(const UChar *, int32_t, int32_t). * @param buffer writable buffer of/for UTF-16 text * @param buffLength length of the current buffer contents @@ -3320,7 +3351,7 @@ public: * @draft ICU 59 */ UnicodeString(wchar_t *buffer, int32_t buffLength, int32_t buffCapacity) : - UnicodeString(static_cast(Char16Ptr(buffer)), buffLength, buffCapacity) {} + UnicodeString(Char16Ptr(buffer).get(), buffLength, buffCapacity) {} #endif /** diff --git a/icu4c/source/common/unistr.cpp b/icu4c/source/common/unistr.cpp index a4d921948f5..c24d8b7f0a9 100644 --- a/icu4c/source/common/unistr.cpp +++ b/icu4c/source/common/unistr.cpp @@ -98,6 +98,48 @@ U_CDECL_END U_NAMESPACE_BEGIN +#ifdef U_ALIASING_BARRIER + +Char16Ptr::Char16Ptr(int null) : p(nullptr) { + U_ASSERT(null == 0); + if (null != 0) { + // Try to provoke a crash. + p = reinterpret_cast(1); + } +} + +ConstChar16Ptr::ConstChar16Ptr(int null) : p(nullptr) { + U_ASSERT(null == 0); + if (null != 0) { + // Try to provoke a crash. + p = reinterpret_cast(1); + } +} + +#else + +Char16Ptr::Char16Ptr(int null) { + U_ASSERT(null == 0); + if (null == 0) { + u.cp = nullptr; + } else { + // Try to provoke a crash. + u.cp = reinterpret_cast(1); + } +} + +ConstChar16Ptr::ConstChar16Ptr(int null) { + U_ASSERT(null == 0); + if (null == 0) { + u.cp = nullptr; + } else { + // Try to provoke a crash. + u.cp = reinterpret_cast(1); + } +} + +#endif + /* The Replaceable virtual destructor can't be defined in the header due to how AIX works with multiple definitions of virtual functions. */ @@ -234,7 +276,7 @@ UnicodeString::UnicodeString(UBool isTerminated, // text is terminated, or else it would have failed the above test textLength = u_strlen(text); } - setArray(const_cast(static_cast(text)), textLength, + setArray(const_cast(text.get()), textLength, isTerminated ? textLength + 1 : textLength); } } diff --git a/icu4c/source/test/intltest/ustrtest.cpp b/icu4c/source/test/intltest/ustrtest.cpp index df579456515..afa503bcbf3 100644 --- a/icu4c/source/test/intltest/ustrtest.cpp +++ b/icu4c/source/test/intltest/ustrtest.cpp @@ -61,6 +61,10 @@ void UnicodeStringTest::runIndexedTest( int32_t index, UBool exec, const char* & TESTCASE_AUTO(TestSizeofUnicodeString); TESTCASE_AUTO(TestStartsWithAndEndsWithNulTerminated); TESTCASE_AUTO(TestMoveSwap); + TESTCASE_AUTO(TestUInt16Pointers); + TESTCASE_AUTO(TestWCharPointers); + TESTCASE_AUTO(TestNullPointers); + TESTCASE_AUTO(TestZeroPointers); TESTCASE_AUTO_END; } @@ -2190,3 +2194,79 @@ UnicodeStringTest::TestMoveSwap() { errln("UnicodeString copy after self-move did not work"); } } + +void +UnicodeStringTest::TestUInt16Pointers() { + static const uint16_t carr[] = { 0x61, 0x62, 0x63, 0 }; + uint16_t arr[4]; + + UnicodeString expected(u"abc"); + assertEquals("abc from pointer", expected, UnicodeString(carr)); + assertEquals("abc from pointer+length", expected, UnicodeString(carr, 3)); + assertEquals("abc from read-only-alias pointer", expected, UnicodeString(TRUE, carr, 3)); + + UnicodeString alias(arr, 0, 4); + alias.append(u'a').append(u'b').append(u'c'); + assertEquals("abc from writable alias", expected, alias); + assertEquals("buffer=abc from writable alias", expected, UnicodeString(arr, 3)); + + UErrorCode errorCode = U_ZERO_ERROR; + int32_t length = UnicodeString(u"def").extract(arr, 4, errorCode); + TEST_ASSERT_STATUS(errorCode); + assertEquals("def from extract()", UnicodeString(u"def"), UnicodeString(arr, length)); +} + +void +UnicodeStringTest::TestWCharPointers() { +#if U_SIZEOF_WCHAR_T==2 + static const wchar_t carr[] = { 0x61, 0x62, 0x63, 0 }; + wchar_t arr[4]; + + UnicodeString expected(u"abc"); + assertEquals("abc from pointer", expected, UnicodeString(carr)); + assertEquals("abc from pointer+length", expected, UnicodeString(carr, 3)); + assertEquals("abc from read-only-alias pointer", expected, UnicodeString(TRUE, carr, 3)); + + UnicodeString alias(arr, 0, 4); + alias.append(u'a').append(u'b').append(u'c'); + assertEquals("abc from writable alias", expected, alias); + assertEquals("buffer=abc from writable alias", expected, UnicodeString(arr, 3)); + + UErrorCode errorCode = U_ZERO_ERROR; + int32_t length = UnicodeString(u"def").extract(arr, 4, errorCode); + TEST_ASSERT_STATUS(errorCode); + assertEquals("def from extract()", UnicodeString(u"def"), UnicodeString(arr, length)); +#endif +} + +void +UnicodeStringTest::TestNullPointers() { + assertTrue("empty from nullptr", UnicodeString(nullptr).isEmpty()); + assertTrue("empty from nullptr+length", UnicodeString(nullptr, 2).isEmpty()); + assertTrue("empty from read-only-alias nullptr", UnicodeString(TRUE, nullptr, 3).isEmpty()); + + UnicodeString alias(nullptr, 4, 4); // empty, no alias + assertTrue("empty from writable alias", alias.isEmpty()); + alias.append(u'a').append(u'b').append(u'c'); + UnicodeString expected(u"abc"); + assertEquals("abc from writable alias", expected, alias); + + UErrorCode errorCode = U_ZERO_ERROR; + UnicodeString(u"def").extract(nullptr, 0, errorCode); + assertEquals("buffer overflow extracting to nullptr", U_BUFFER_OVERFLOW_ERROR, errorCode); +} + +void +UnicodeStringTest::TestZeroPointers() { + // There are constructor overloads with one and three integer parameters + // which match passing 0, so we cannot test using 0 for UnicodeString(pointer) + // or UnicodeString(read-only or writable alias). + // There are multiple two-parameter constructors that make using 0 + // for the first parameter ambiguous already, + // so we cannot test using 0 for UnicodeString(pointer, length). + + // extract() also has enough overloads to be ambiguous with 0. + // Test the pointer wrapper directly. + assertTrue("0 --> nullptr", Char16Ptr(0).get() == nullptr); + assertTrue("0 --> const nullptr", ConstChar16Ptr(0).get() == nullptr); +} diff --git a/icu4c/source/test/intltest/ustrtest.h b/icu4c/source/test/intltest/ustrtest.h index 0c449350f01..a2e2fbd4b71 100644 --- a/icu4c/source/test/intltest/ustrtest.h +++ b/icu4c/source/test/intltest/ustrtest.h @@ -92,6 +92,11 @@ public: void TestUnicodeStringImplementsAppendable(); void TestSizeofUnicodeString(); void TestMoveSwap(); + + void TestUInt16Pointers(); + void TestWCharPointers(); + void TestNullPointers(); + void TestZeroPointers(); }; #endif