From 9fb93edd5bd4531f72faadfb269a0aa25d413b5d Mon Sep 17 00:00:00 2001 From: Vic Yang Date: Thu, 5 Sep 2019 13:18:27 -0700 Subject: [PATCH] Reland "libutils: Introduce StaticString16"" This reverts commit 1270e4fbf17402fb01dbfa7928a654d41d8553ee. Bug: 138856262 Test: Run unit tests. Change-Id: I37be01d7d4f98a83078870cb0917275336fa2bbd Merged-In: I8da93f2c9b95183e32d4a2ea895f90c449abbe4d --- libutils/Android.bp | 1 + libutils/SharedBuffer.cpp | 1 + libutils/SharedBuffer.h | 7 +- libutils/String16.cpp | 157 ++++++++++++++------- libutils/String16_test.cpp | 218 ++++++++++++++++++++++++++++++ libutils/include/utils/String16.h | 114 +++++++++++++++- 6 files changed, 448 insertions(+), 50 deletions(-) create mode 100644 libutils/String16_test.cpp diff --git a/libutils/Android.bp b/libutils/Android.bp index 8be4dd088..30798dd54 100644 --- a/libutils/Android.bp +++ b/libutils/Android.bp @@ -205,6 +205,7 @@ cc_test { "Mutex_test.cpp", "SharedBuffer_test.cpp", "String8_test.cpp", + "String16_test.cpp", "StrongPointer_test.cpp", "Unicode_test.cpp", "Vector_test.cpp", diff --git a/libutils/SharedBuffer.cpp b/libutils/SharedBuffer.cpp index 7910c6e0e..3e703dbc8 100644 --- a/libutils/SharedBuffer.cpp +++ b/libutils/SharedBuffer.cpp @@ -41,6 +41,7 @@ SharedBuffer* SharedBuffer::alloc(size_t size) // The following is OK on Android-supported platforms. sb->mRefs.store(1, std::memory_order_relaxed); sb->mSize = size; + sb->mClientMetadata = 0; } return sb; } diff --git a/libutils/SharedBuffer.h b/libutils/SharedBuffer.h index fdf13a9e7..476c842fe 100644 --- a/libutils/SharedBuffer.h +++ b/libutils/SharedBuffer.h @@ -102,7 +102,12 @@ private: // Must be sized to preserve correct alignment. mutable std::atomic mRefs; size_t mSize; - uint32_t mReserved[2]; + uint32_t mReserved; +public: + // mClientMetadata is reserved for client use. It is initialized to 0 + // and the clients can do whatever they want with it. Note that this is + // placed last so that it is adjcent to the buffer allocated. + uint32_t mClientMetadata; }; static_assert(sizeof(SharedBuffer) % 8 == 0 diff --git a/libutils/String16.cpp b/libutils/String16.cpp index 818b17124..5c3cf3261 100644 --- a/libutils/String16.cpp +++ b/libutils/String16.cpp @@ -24,21 +24,21 @@ namespace android { +static const StaticString16 emptyString(u""); static inline char16_t* getEmptyString() { - static SharedBuffer* gEmptyStringBuf = [] { - SharedBuffer* buf = SharedBuffer::alloc(sizeof(char16_t)); - char16_t* str = static_cast(buf->data()); - *str = 0; - return buf; - }(); - - gEmptyStringBuf->acquire(); - return static_cast(gEmptyStringBuf->data()); + return const_cast(emptyString.string()); } // --------------------------------------------------------------------------- -static char16_t* allocFromUTF8(const char* u8str, size_t u8len) +void* String16::alloc(size_t size) +{ + SharedBuffer* buf = SharedBuffer::alloc(size); + buf->mClientMetadata = kIsSharedBufferAllocated; + return buf; +} + +char16_t* String16::allocFromUTF8(const char* u8str, size_t u8len) { if (u8len == 0) return getEmptyString(); @@ -49,7 +49,7 @@ static char16_t* allocFromUTF8(const char* u8str, size_t u8len) return getEmptyString(); } - SharedBuffer* buf = SharedBuffer::alloc(sizeof(char16_t)*(u16len+1)); + SharedBuffer* buf = static_cast(alloc(sizeof(char16_t) * (u16len + 1))); if (buf) { u8cur = (const uint8_t*) u8str; char16_t* u16str = (char16_t*)buf->data(); @@ -66,13 +66,13 @@ static char16_t* allocFromUTF8(const char* u8str, size_t u8len) return getEmptyString(); } -static char16_t* allocFromUTF16(const char16_t* u16str, size_t u16len) { +char16_t* String16::allocFromUTF16(const char16_t* u16str, size_t u16len) { if (u16len >= SIZE_MAX / sizeof(char16_t)) { android_errorWriteLog(0x534e4554, "73826242"); abort(); } - SharedBuffer* buf = SharedBuffer::alloc((u16len + 1) * sizeof(char16_t)); + SharedBuffer* buf = static_cast(alloc((u16len + 1) * sizeof(char16_t))); ALOG_ASSERT(buf, "Unable to allocate shared buffer"); if (buf) { char16_t* str = (char16_t*)buf->data(); @@ -97,8 +97,8 @@ String16::String16(StaticLinkage) // having run. In this case we always allocate an empty string. It's less // efficient than using getEmptyString(), but we assume it's uncommon. - char16_t* data = static_cast( - SharedBuffer::alloc(sizeof(char16_t))->data()); + SharedBuffer* buf = static_cast(alloc(sizeof(char16_t))); + char16_t* data = static_cast(buf->data()); data[0] = 0; mString = data; } @@ -106,7 +106,7 @@ String16::String16(StaticLinkage) String16::String16(const String16& o) : mString(o.mString) { - SharedBuffer::bufferFromData(mString)->acquire(); + acquire(); } String16::String16(const String16& o, size_t len, size_t begin) @@ -136,26 +136,30 @@ String16::String16(const char* o, size_t len) String16::~String16() { - SharedBuffer::bufferFromData(mString)->release(); + release(); } size_t String16::size() const { - return SharedBuffer::sizeFromData(mString)/sizeof(char16_t)-1; + if (isStaticString()) { + return staticStringSize(); + } else { + return SharedBuffer::sizeFromData(mString) / sizeof(char16_t) - 1; + } } void String16::setTo(const String16& other) { - SharedBuffer::bufferFromData(other.mString)->acquire(); - SharedBuffer::bufferFromData(mString)->release(); + release(); mString = other.mString; + acquire(); } status_t String16::setTo(const String16& other, size_t len, size_t begin) { const size_t N = other.size(); if (begin >= N) { - SharedBuffer::bufferFromData(mString)->release(); + release(); mString = getEmptyString(); return OK; } @@ -184,8 +188,7 @@ status_t String16::setTo(const char16_t* other, size_t len) abort(); } - SharedBuffer* buf = SharedBuffer::bufferFromData(mString) - ->editResize((len+1)*sizeof(char16_t)); + SharedBuffer* buf = static_cast(editResize((len + 1) * sizeof(char16_t))); if (buf) { char16_t* str = (char16_t*)buf->data(); memmove(str, other, len*sizeof(char16_t)); @@ -212,8 +215,8 @@ status_t String16::append(const String16& other) abort(); } - SharedBuffer* buf = SharedBuffer::bufferFromData(mString) - ->editResize((myLen+otherLen+1)*sizeof(char16_t)); + SharedBuffer* buf = + static_cast(editResize((myLen + otherLen + 1) * sizeof(char16_t))); if (buf) { char16_t* str = (char16_t*)buf->data(); memcpy(str+myLen, other, (otherLen+1)*sizeof(char16_t)); @@ -238,8 +241,8 @@ status_t String16::append(const char16_t* chrs, size_t otherLen) abort(); } - SharedBuffer* buf = SharedBuffer::bufferFromData(mString) - ->editResize((myLen+otherLen+1)*sizeof(char16_t)); + SharedBuffer* buf = + static_cast(editResize((myLen + otherLen + 1) * sizeof(char16_t))); if (buf) { char16_t* str = (char16_t*)buf->data(); memcpy(str+myLen, chrs, otherLen*sizeof(char16_t)); @@ -273,8 +276,8 @@ status_t String16::insert(size_t pos, const char16_t* chrs, size_t len) len, myLen, String8(chrs, len).string()); #endif - SharedBuffer* buf = SharedBuffer::bufferFromData(mString) - ->editResize((myLen+len+1)*sizeof(char16_t)); + SharedBuffer* buf = + static_cast(editResize((myLen + len + 1) * sizeof(char16_t))); if (buf) { char16_t* str = (char16_t*)buf->data(); if (pos < myLen) { @@ -338,23 +341,87 @@ bool String16::contains(const char16_t* chrs) const return strstr16(mString, chrs) != nullptr; } +void* String16::edit() { + SharedBuffer* buf; + if (isStaticString()) { + buf = static_cast(alloc((size() + 1) * sizeof(char16_t))); + if (buf) { + buf->acquire(); + memcpy(buf->data(), mString, (size() + 1) * sizeof(char16_t)); + } + } else { + buf = SharedBuffer::bufferFromData(mString)->edit(); + buf->mClientMetadata = kIsSharedBufferAllocated; + } + return buf; +} + +void* String16::editResize(size_t newSize) { + SharedBuffer* buf; + if (isStaticString()) { + size_t copySize = (size() + 1) * sizeof(char16_t); + if (newSize < copySize) { + copySize = newSize; + } + buf = static_cast(alloc(newSize)); + if (buf) { + buf->acquire(); + memcpy(buf->data(), mString, copySize); + } + } else { + buf = SharedBuffer::bufferFromData(mString)->editResize(newSize); + buf->mClientMetadata = kIsSharedBufferAllocated; + } + return buf; +} + +void String16::acquire() +{ + if (!isStaticString()) { + SharedBuffer::bufferFromData(mString)->acquire(); + } +} + +void String16::release() +{ + if (!isStaticString()) { + SharedBuffer::bufferFromData(mString)->release(); + } +} + +bool String16::isStaticString() const { + // See String16.h for notes on the memory layout of String16::StaticData and + // SharedBuffer. + static_assert(sizeof(SharedBuffer) - offsetof(SharedBuffer, mClientMetadata) == 4); + const uint32_t* p = reinterpret_cast(mString); + return (*(p - 1) & kIsSharedBufferAllocated) == 0; +} + +size_t String16::staticStringSize() const { + // See String16.h for notes on the memory layout of String16::StaticData and + // SharedBuffer. + static_assert(sizeof(SharedBuffer) - offsetof(SharedBuffer, mClientMetadata) == 4); + const uint32_t* p = reinterpret_cast(mString); + return static_cast(*(p - 1)); +} + status_t String16::makeLower() { const size_t N = size(); const char16_t* str = string(); - char16_t* edit = nullptr; + char16_t* edited = nullptr; for (size_t i=0; i= 'A' && v <= 'Z') { - if (!edit) { - SharedBuffer* buf = SharedBuffer::bufferFromData(mString)->edit(); + if (!edited) { + SharedBuffer* buf = static_cast(edit()); if (!buf) { return NO_MEMORY; } - edit = (char16_t*)buf->data(); - mString = str = edit; + edited = (char16_t*)buf->data(); + mString = str = edited; } - edit[i] = tolower((char)v); + edited[i] = tolower((char)v); } } return OK; @@ -364,18 +431,18 @@ status_t String16::replaceAll(char16_t replaceThis, char16_t withThis) { const size_t N = size(); const char16_t* str = string(); - char16_t* edit = nullptr; + char16_t* edited = nullptr; for (size_t i=0; iedit(); + if (!edited) { + SharedBuffer* buf = static_cast(edit()); if (!buf) { return NO_MEMORY; } - edit = (char16_t*)buf->data(); - mString = str = edit; + edited = (char16_t*)buf->data(); + mString = str = edited; } - edit[i] = withThis; + edited[i] = withThis; } } return OK; @@ -385,7 +452,7 @@ status_t String16::remove(size_t len, size_t begin) { const size_t N = size(); if (begin >= N) { - SharedBuffer::bufferFromData(mString)->release(); + release(); mString = getEmptyString(); return OK; } @@ -395,8 +462,7 @@ status_t String16::remove(size_t len, size_t begin) } if (begin > 0) { - SharedBuffer* buf = SharedBuffer::bufferFromData(mString) - ->editResize((N+1)*sizeof(char16_t)); + SharedBuffer* buf = static_cast(editResize((N + 1) * sizeof(char16_t))); if (!buf) { return NO_MEMORY; } @@ -404,8 +470,7 @@ status_t String16::remove(size_t len, size_t begin) memmove(str, str+begin, (N-begin+1)*sizeof(char16_t)); mString = str; } - SharedBuffer* buf = SharedBuffer::bufferFromData(mString) - ->editResize((len+1)*sizeof(char16_t)); + SharedBuffer* buf = static_cast(editResize((len + 1) * sizeof(char16_t))); if (buf) { char16_t* str = (char16_t*)buf->data(); str[len] = 0; diff --git a/libutils/String16_test.cpp b/libutils/String16_test.cpp new file mode 100644 index 000000000..f1f24c394 --- /dev/null +++ b/libutils/String16_test.cpp @@ -0,0 +1,218 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#include + +namespace android { + +::testing::AssertionResult Char16_tStringEquals(const char16_t* a, const char16_t* b) { + if (strcmp16(a, b) != 0) { + return ::testing::AssertionFailure() + << "\"" << String8(a).c_str() << "\" not equal to \"" << String8(b).c_str() << "\""; + } + return ::testing::AssertionSuccess(); +} + +#define EXPECT_STR16EQ(a, b) EXPECT_TRUE(Char16_tStringEquals(a, b)) + +TEST(String16Test, FromChar16_t) { + String16 tmp(u"Verify me"); + EXPECT_STR16EQ(u"Verify me", tmp); +} + +TEST(String16Test, FromChar16_tSized) { + String16 tmp(u"Verify me", 7); + EXPECT_STR16EQ(u"Verify ", tmp); +} + +TEST(String16Test, FromChar) { + String16 tmp("Verify me"); + EXPECT_STR16EQ(u"Verify me", tmp); +} + +TEST(String16Test, FromCharSized) { + String16 tmp("Verify me", 7); + EXPECT_STR16EQ(u"Verify ", tmp); +} + +TEST(String16Test, Copy) { + String16 tmp("Verify me"); + String16 another = tmp; + EXPECT_STR16EQ(u"Verify me", tmp); + EXPECT_STR16EQ(u"Verify me", another); +} + +TEST(String16Test, Move) { + String16 tmp("Verify me"); + String16 another(std::move(tmp)); + EXPECT_STR16EQ(u"Verify me", another); +} + +TEST(String16Test, Size) { + String16 tmp("Verify me"); + EXPECT_EQ(9U, tmp.size()); +} + +TEST(String16Test, setTo) { + String16 tmp("Verify me"); + tmp.setTo(u"New content"); + EXPECT_EQ(11U, tmp.size()); + EXPECT_STR16EQ(u"New content", tmp); +} + +TEST(String16Test, Append) { + String16 tmp("Verify me"); + tmp.append(String16("Hello")); + EXPECT_EQ(14U, tmp.size()); + EXPECT_STR16EQ(u"Verify meHello", tmp); +} + +TEST(String16Test, Insert) { + String16 tmp("Verify me"); + tmp.insert(6, u"Insert"); + EXPECT_EQ(15U, tmp.size()); + EXPECT_STR16EQ(u"VerifyInsert me", tmp); +} + +TEST(String16Test, Remove) { + String16 tmp("Verify me"); + tmp.remove(2, 6); + EXPECT_EQ(2U, tmp.size()); + EXPECT_STR16EQ(u" m", tmp); +} + +TEST(String16Test, MakeLower) { + String16 tmp("Verify Me!"); + tmp.makeLower(); + EXPECT_EQ(10U, tmp.size()); + EXPECT_STR16EQ(u"verify me!", tmp); +} + +TEST(String16Test, ReplaceAll) { + String16 tmp("Verify verify Verify"); + tmp.replaceAll(u'r', u'!'); + EXPECT_STR16EQ(u"Ve!ify ve!ify Ve!ify", tmp); +} + +TEST(String16Test, Compare) { + String16 tmp("Verify me"); + EXPECT_EQ(String16(u"Verify me"), tmp); +} + +TEST(String16Test, StaticString) { + String16 nonStaticString("NonStatic"); + StaticString16 staticString(u"Static"); + + EXPECT_TRUE(staticString.isStaticString()); + EXPECT_FALSE(nonStaticString.isStaticString()); +} + +TEST(String16Test, StaticStringCopy) { + StaticString16 tmp(u"Verify me"); + String16 another = tmp; + EXPECT_STR16EQ(u"Verify me", tmp); + EXPECT_STR16EQ(u"Verify me", another); + EXPECT_TRUE(tmp.isStaticString()); + EXPECT_TRUE(another.isStaticString()); +} + +TEST(String16Test, StaticStringMove) { + StaticString16 tmp(u"Verify me"); + String16 another(std::move(tmp)); + EXPECT_STR16EQ(u"Verify me", another); + EXPECT_TRUE(another.isStaticString()); +} + +TEST(String16Test, StaticStringSize) { + StaticString16 tmp(u"Verify me"); + EXPECT_EQ(9U, tmp.size()); +} + +TEST(String16Test, StaticStringSetTo) { + StaticString16 tmp(u"Verify me"); + tmp.setTo(u"New content"); + EXPECT_EQ(11U, tmp.size()); + EXPECT_STR16EQ(u"New content", tmp); + EXPECT_FALSE(tmp.isStaticString()); +} + +TEST(String16Test, StaticStringAppend) { + StaticString16 tmp(u"Verify me"); + tmp.append(String16("Hello")); + EXPECT_EQ(14U, tmp.size()); + EXPECT_STR16EQ(u"Verify meHello", tmp); + EXPECT_FALSE(tmp.isStaticString()); +} + +TEST(String16Test, StaticStringInsert) { + StaticString16 tmp(u"Verify me"); + tmp.insert(6, u"Insert"); + EXPECT_EQ(15U, tmp.size()); + EXPECT_STR16EQ(u"VerifyInsert me", tmp); + EXPECT_FALSE(tmp.isStaticString()); +} + +TEST(String16Test, StaticStringRemove) { + StaticString16 tmp(u"Verify me"); + tmp.remove(2, 6); + EXPECT_EQ(2U, tmp.size()); + EXPECT_STR16EQ(u" m", tmp); + EXPECT_FALSE(tmp.isStaticString()); +} + +TEST(String16Test, StaticStringMakeLower) { + StaticString16 tmp(u"Verify me!"); + tmp.makeLower(); + EXPECT_EQ(10U, tmp.size()); + EXPECT_STR16EQ(u"verify me!", tmp); + EXPECT_FALSE(tmp.isStaticString()); +} + +TEST(String16Test, StaticStringReplaceAll) { + StaticString16 tmp(u"Verify verify Verify"); + tmp.replaceAll(u'r', u'!'); + EXPECT_STR16EQ(u"Ve!ify ve!ify Ve!ify", tmp); + EXPECT_FALSE(tmp.isStaticString()); +} + +TEST(String16Test, StaticStringCompare) { + StaticString16 tmp(u"Verify me"); + EXPECT_EQ(String16(u"Verify me"), tmp); +} + +TEST(String16Test, StringSetToStaticString) { + StaticString16 tmp(u"Verify me"); + String16 another(u"nonstatic"); + another = tmp; + EXPECT_STR16EQ(u"Verify me", tmp); + EXPECT_STR16EQ(u"Verify me", another); +} + +TEST(String16Test, StringMoveFromStaticString) { + StaticString16 tmp(u"Verify me"); + String16 another(std::move(tmp)); + EXPECT_STR16EQ(u"Verify me", another); +} + +TEST(String16Test, EmptyStringIsStatic) { + String16 tmp(""); + EXPECT_TRUE(tmp.isStaticString()); +} + +} // namespace android diff --git a/libutils/include/utils/String16.h b/libutils/include/utils/String16.h index afbc2edc5..adc3e7de9 100644 --- a/libutils/include/utils/String16.h +++ b/libutils/include/utils/String16.h @@ -37,13 +37,17 @@ namespace android { class String8; +template +class StaticString16; + // DO NOT USE: please use std::u16string //! This is a string holding UTF-16 characters. class String16 { public: - /* use String16(StaticLinkage) if you're statically linking against + /* + * Use String16(StaticLinkage) if you're statically linking against * libutils and declaring an empty static String16, e.g.: * * static String16 sAStaticEmptyString(String16::kEmptyString); @@ -123,14 +127,118 @@ public: inline operator const char16_t*() const; -private: - const char16_t* mString; + // Static and non-static String16 behave the same for the users, so + // this method isn't of much use for the users. It is public for testing. + bool isStaticString() const; + + private: + /* + * A flag indicating the type of underlying buffer. + */ + static constexpr uint32_t kIsSharedBufferAllocated = 0x80000000; + + /* + * alloc() returns void* so that SharedBuffer class is not exposed. + */ + static void* alloc(size_t size); + static char16_t* allocFromUTF8(const char* u8str, size_t u8len); + static char16_t* allocFromUTF16(const char16_t* u16str, size_t u16len); + + /* + * edit() and editResize() return void* so that SharedBuffer class + * is not exposed. + */ + void* edit(); + void* editResize(size_t new_size); + + void acquire(); + void release(); + + size_t staticStringSize() const; + + const char16_t* mString; + +protected: + /* + * Data structure used to allocate static storage for static String16. + * + * Note that this data structure and SharedBuffer are used interchangably + * as the underlying data structure for a String16. Therefore, the layout + * of this data structure must match the part in SharedBuffer that is + * visible to String16. + */ + template + struct StaticData { + // The high bit of 'size' is used as a flag. + static_assert(N - 1 < kIsSharedBufferAllocated, "StaticString16 too long!"); + constexpr StaticData() : size(N - 1), data{0} {} + const uint32_t size; + char16_t data[N]; + + constexpr StaticData(const StaticData&) = default; + }; + + /* + * Helper function for constructing a StaticData object. + */ + template + static constexpr const StaticData makeStaticData(const char16_t (&s)[N]) { + StaticData r; + // The 'size' field is at the same location where mClientMetadata would + // be for a SharedBuffer. We do NOT set kIsSharedBufferAllocated flag + // here. + for (size_t i = 0; i < N - 1; ++i) r.data[i] = s[i]; + return r; + } + + template + explicit constexpr String16(const StaticData& s) : mString(s.data) {} + +public: + template + explicit constexpr String16(const StaticString16& s) : mString(s.mString) {} }; // String16 can be trivially moved using memcpy() because moving does not // require any change to the underlying SharedBuffer contents or reference count. ANDROID_TRIVIAL_MOVE_TRAIT(String16) +// --------------------------------------------------------------------------- + +/* + * A StaticString16 object is a specialized String16 object. Instead of holding + * the string data in a ref counted SharedBuffer object, it holds data in a + * buffer within StaticString16 itself. Note that this buffer is NOT ref + * counted and is assumed to be available for as long as there is at least a + * String16 object using it. Therefore, one must be extra careful to NEVER + * assign a StaticString16 to a String16 that outlives the StaticString16 + * object. + * + * THE SAFEST APPROACH IS TO USE StaticString16 ONLY AS GLOBAL VARIABLES. + * + * A StaticString16 SHOULD NEVER APPEAR IN APIs. USE String16 INSTEAD. + */ +template +class StaticString16 : public String16 { +public: + constexpr StaticString16(const char16_t (&s)[N]) : String16(mData), mData(makeStaticData(s)) {} + + constexpr StaticString16(const StaticString16& other) + : String16(mData), mData(other.mData) {} + + constexpr StaticString16(const StaticString16&&) = delete; + + // There is no reason why one would want to 'new' a StaticString16. Delete + // it to discourage misuse. + static void* operator new(std::size_t) = delete; + +private: + const StaticData mData; +}; + +template +StaticString16(const F&)->StaticString16; + // --------------------------------------------------------------------------- // No user servicable parts below.