diff --git a/libutils/Android.bp b/libutils/Android.bp index 30798dd54..8be4dd088 100644 --- a/libutils/Android.bp +++ b/libutils/Android.bp @@ -205,7 +205,6 @@ 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 3e703dbc8..7910c6e0e 100644 --- a/libutils/SharedBuffer.cpp +++ b/libutils/SharedBuffer.cpp @@ -41,7 +41,6 @@ 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 476c842fe..fdf13a9e7 100644 --- a/libutils/SharedBuffer.h +++ b/libutils/SharedBuffer.h @@ -102,12 +102,7 @@ private: // Must be sized to preserve correct alignment. mutable std::atomic mRefs; size_t mSize; - 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; + uint32_t mReserved[2]; }; static_assert(sizeof(SharedBuffer) % 8 == 0 diff --git a/libutils/String16.cpp b/libutils/String16.cpp index 5c3cf3261..818b17124 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() { - return const_cast(emptyString.string()); + 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()); } // --------------------------------------------------------------------------- -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) +static char16_t* allocFromUTF8(const char* u8str, size_t u8len) { if (u8len == 0) return getEmptyString(); @@ -49,7 +49,7 @@ char16_t* String16::allocFromUTF8(const char* u8str, size_t u8len) return getEmptyString(); } - SharedBuffer* buf = static_cast(alloc(sizeof(char16_t) * (u16len + 1))); + SharedBuffer* buf = SharedBuffer::alloc(sizeof(char16_t)*(u16len+1)); if (buf) { u8cur = (const uint8_t*) u8str; char16_t* u16str = (char16_t*)buf->data(); @@ -66,13 +66,13 @@ char16_t* String16::allocFromUTF8(const char* u8str, size_t u8len) return getEmptyString(); } -char16_t* String16::allocFromUTF16(const char16_t* u16str, size_t u16len) { +static char16_t* allocFromUTF16(const char16_t* u16str, size_t u16len) { if (u16len >= SIZE_MAX / sizeof(char16_t)) { android_errorWriteLog(0x534e4554, "73826242"); abort(); } - SharedBuffer* buf = static_cast(alloc((u16len + 1) * sizeof(char16_t))); + SharedBuffer* buf = SharedBuffer::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. - SharedBuffer* buf = static_cast(alloc(sizeof(char16_t))); - char16_t* data = static_cast(buf->data()); + char16_t* data = static_cast( + SharedBuffer::alloc(sizeof(char16_t))->data()); data[0] = 0; mString = data; } @@ -106,7 +106,7 @@ String16::String16(StaticLinkage) String16::String16(const String16& o) : mString(o.mString) { - acquire(); + SharedBuffer::bufferFromData(mString)->acquire(); } String16::String16(const String16& o, size_t len, size_t begin) @@ -136,30 +136,26 @@ String16::String16(const char* o, size_t len) String16::~String16() { - release(); + SharedBuffer::bufferFromData(mString)->release(); } size_t String16::size() const { - if (isStaticString()) { - return staticStringSize(); - } else { - return SharedBuffer::sizeFromData(mString) / sizeof(char16_t) - 1; - } + return SharedBuffer::sizeFromData(mString)/sizeof(char16_t)-1; } void String16::setTo(const String16& other) { - release(); + SharedBuffer::bufferFromData(other.mString)->acquire(); + SharedBuffer::bufferFromData(mString)->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) { - release(); + SharedBuffer::bufferFromData(mString)->release(); mString = getEmptyString(); return OK; } @@ -188,7 +184,8 @@ status_t String16::setTo(const char16_t* other, size_t len) abort(); } - SharedBuffer* buf = static_cast(editResize((len + 1) * sizeof(char16_t))); + SharedBuffer* buf = SharedBuffer::bufferFromData(mString) + ->editResize((len+1)*sizeof(char16_t)); if (buf) { char16_t* str = (char16_t*)buf->data(); memmove(str, other, len*sizeof(char16_t)); @@ -215,8 +212,8 @@ status_t String16::append(const String16& other) abort(); } - SharedBuffer* buf = - static_cast(editResize((myLen + otherLen + 1) * sizeof(char16_t))); + SharedBuffer* buf = SharedBuffer::bufferFromData(mString) + ->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)); @@ -241,8 +238,8 @@ status_t String16::append(const char16_t* chrs, size_t otherLen) abort(); } - SharedBuffer* buf = - static_cast(editResize((myLen + otherLen + 1) * sizeof(char16_t))); + SharedBuffer* buf = SharedBuffer::bufferFromData(mString) + ->editResize((myLen+otherLen+1)*sizeof(char16_t)); if (buf) { char16_t* str = (char16_t*)buf->data(); memcpy(str+myLen, chrs, otherLen*sizeof(char16_t)); @@ -276,8 +273,8 @@ status_t String16::insert(size_t pos, const char16_t* chrs, size_t len) len, myLen, String8(chrs, len).string()); #endif - SharedBuffer* buf = - static_cast(editResize((myLen + len + 1) * sizeof(char16_t))); + SharedBuffer* buf = SharedBuffer::bufferFromData(mString) + ->editResize((myLen+len+1)*sizeof(char16_t)); if (buf) { char16_t* str = (char16_t*)buf->data(); if (pos < myLen) { @@ -341,87 +338,23 @@ 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* edited = nullptr; + char16_t* edit = nullptr; for (size_t i=0; i= 'A' && v <= 'Z') { - if (!edited) { - SharedBuffer* buf = static_cast(edit()); + if (!edit) { + SharedBuffer* buf = SharedBuffer::bufferFromData(mString)->edit(); if (!buf) { return NO_MEMORY; } - edited = (char16_t*)buf->data(); - mString = str = edited; + edit = (char16_t*)buf->data(); + mString = str = edit; } - edited[i] = tolower((char)v); + edit[i] = tolower((char)v); } } return OK; @@ -431,18 +364,18 @@ status_t String16::replaceAll(char16_t replaceThis, char16_t withThis) { const size_t N = size(); const char16_t* str = string(); - char16_t* edited = nullptr; + char16_t* edit = nullptr; for (size_t i=0; i(edit()); + if (!edit) { + SharedBuffer* buf = SharedBuffer::bufferFromData(mString)->edit(); if (!buf) { return NO_MEMORY; } - edited = (char16_t*)buf->data(); - mString = str = edited; + edit = (char16_t*)buf->data(); + mString = str = edit; } - edited[i] = withThis; + edit[i] = withThis; } } return OK; @@ -452,7 +385,7 @@ status_t String16::remove(size_t len, size_t begin) { const size_t N = size(); if (begin >= N) { - release(); + SharedBuffer::bufferFromData(mString)->release(); mString = getEmptyString(); return OK; } @@ -462,7 +395,8 @@ status_t String16::remove(size_t len, size_t begin) } if (begin > 0) { - SharedBuffer* buf = static_cast(editResize((N + 1) * sizeof(char16_t))); + SharedBuffer* buf = SharedBuffer::bufferFromData(mString) + ->editResize((N+1)*sizeof(char16_t)); if (!buf) { return NO_MEMORY; } @@ -470,7 +404,8 @@ status_t String16::remove(size_t len, size_t begin) memmove(str, str+begin, (N-begin+1)*sizeof(char16_t)); mString = str; } - SharedBuffer* buf = static_cast(editResize((len + 1) * sizeof(char16_t))); + SharedBuffer* buf = SharedBuffer::bufferFromData(mString) + ->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 deleted file mode 100644 index f1f24c394..000000000 --- a/libutils/String16_test.cpp +++ /dev/null @@ -1,218 +0,0 @@ -/* - * 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 adc3e7de9..afbc2edc5 100644 --- a/libutils/include/utils/String16.h +++ b/libutils/include/utils/String16.h @@ -37,17 +37,13 @@ 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); @@ -127,118 +123,14 @@ public: inline operator const char16_t*() const; - // 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) {} +private: + const char16_t* 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.