feedc0de/qt-creator
1
0
Fork 0
forked from qt-creator/qt-creator
Code Pull Requests Activity

Utils: Remove null termination from SmallString

Browse Source 
We expect mostly a string view which has no null termination. Code which
depends on null termination is broken and it is better break early.

Change-Id: I7c2c41fb114e6aaf3a23053b522b37f7af5e1abf
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>
This commit is contained in:
Marco Bubke
2022-08-07 03:37:47 +02:00
parent cf064500aa
commit c9d45d1bc0
5 changed files with 28 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/libs/sqlite/sqlitedatabasebackend.cpp
View File

@@ -110,7 +110,10 @@ void DatabaseBackend::open(Utils::SmallStringView databaseFilePath, OpenMode mod
{ {
checkCanOpenDatabase(databaseFilePath); checkCanOpenDatabase(databaseFilePath);
int resultCode = sqlite3_open_v2(databaseFilePath.data(), &m_databaseHandle, openMode(mode), nullptr); int resultCode = sqlite3_open_v2(std::string(databaseFilePath).c_str(),
&m_databaseHandle,
openMode(mode),
nullptr);
checkDatabaseCouldBeOpened(resultCode); checkDatabaseCouldBeOpened(resultCode);

4
src/libs/sqlite/sqlitesessions.cpp
View File

@@ -73,7 +73,7 @@ int xConflict(void *, int conflict, sqlite3_changeset_iter *)
void Sessions::attachTables(const Utils::SmallStringVector &tableNames) void Sessions::attachTables(const Utils::SmallStringVector &tableNames)
{ {
for (Utils::SmallStringView tableName : tableNames) { for (Utils::SmallStringView tableName : tableNames) {
int resultCode = sqlite3session_attach(session.get(), tableName.data()); int resultCode = sqlite3session_attach(session.get(), std::string(tableName).c_str());
checkResultCode(resultCode); checkResultCode(resultCode);
} }
} }
@@ -89,7 +89,7 @@ void Sessions::create()
{ {
sqlite3_session *newSession = nullptr; sqlite3_session *newSession = nullptr;
int resultCode = sqlite3session_create(database.backend().sqliteDatabaseHandle(), int resultCode = sqlite3session_create(database.backend().sqliteDatabaseHandle(),
databaseName.data(), std::string(databaseName.data()).c_str(),
&newSession); &newSession);
session.reset(newSession); session.reset(newSession);

16
src/libs/utils/smallstring.h
View File

@@ -103,7 +103,7 @@ public:
m_data.control.setIsShortString(true); m_data.control.setIsShortString(true);
m_data.control.setIsReadOnlyReference(false); m_data.control.setIsReadOnlyReference(false);
} else { } else {
m_data.reference.pointer = Memory::allocate(capacity + 1); m_data.reference.pointer = Memory::allocate(capacity);
std::char_traits<char>::copy(m_data.reference.pointer, string, size); std::char_traits<char>::copy(m_data.reference.pointer, string, size);
initializeLongString(size, capacity); initializeLongString(size, capacity);
} }
@@ -274,8 +274,7 @@ public:
{ {
if (fitsNotInCapacity(newCapacity)) { if (fitsNotInCapacity(newCapacity)) {
if (Q_UNLIKELY(hasAllocatedMemory())) { if (Q_UNLIKELY(hasAllocatedMemory())) {
m_data.reference.pointer = Memory::reallocate(m_data.reference.pointer, m_data.reference.pointer = Memory::reallocate(m_data.reference.pointer, newCapacity);
newCapacity + 1);
m_data.reference.capacity = newCapacity; m_data.reference.capacity = newCapacity;
} else if (newCapacity <= shortStringCapacity()) { } else if (newCapacity <= shortStringCapacity()) {
new (this) BasicSmallString{m_data.reference.pointer, m_data.reference.size}; new (this) BasicSmallString{m_data.reference.pointer, m_data.reference.size};
@@ -284,7 +283,7 @@ public:
newCapacity = std::max(newCapacity, oldSize); newCapacity = std::max(newCapacity, oldSize);
const char *oldData = data(); const char *oldData = data();
char *newData = Memory::allocate(newCapacity + 1); char *newData = Memory::allocate(newCapacity);
std::char_traits<char>::copy(newData, oldData, oldSize); std::char_traits<char>::copy(newData, oldData, oldSize);
m_data.reference.pointer = newData; m_data.reference.pointer = newData;
initializeLongString(oldSize, newCapacity); initializeLongString(oldSize, newCapacity);
@@ -296,7 +295,6 @@ public:
{ {
reserve(newSize); reserve(newSize);
setSize(newSize); setSize(newSize);
at(newSize) = 0;
} }
void clear() noexcept void clear() noexcept
@@ -487,7 +485,6 @@ public:
reserve(optimalCapacity(newSize)); reserve(optimalCapacity(newSize));
std::char_traits<char>::copy(data() + oldSize, string.data(), string.size()); std::char_traits<char>::copy(data() + oldSize, string.data(), string.size());
at(newSize) = 0;
setSize(newSize); setSize(newSize);
} }
@@ -520,7 +517,6 @@ public:
newEnd = data() + newSize; newEnd = data() + newSize;
} }
*newEnd = 0;
setSize(newEnd - data()); setSize(newEnd - data());
} }
@@ -598,7 +594,7 @@ public:
size_type optimalCapacity(const size_type size) size_type optimalCapacity(const size_type size)
{ {
if (fitsNotInCapacity(size)) if (fitsNotInCapacity(size))
return optimalHeapCapacity(size + 1) - 1; return optimalHeapCapacity(size);
return size; return size;
} }
@@ -792,8 +788,6 @@ private:
std::memcpy(currentData, string.data(), string.size()); std::memcpy(currentData, string.data(), string.size());
currentData += string.size(); currentData += string.size();
} }
at(size) = 0;
} }
constexpr void initializeLongString(size_type size, size_type capacity) constexpr void initializeLongString(size_type size, size_type capacity)
@@ -877,7 +871,6 @@ private:
newSize -= sizeDifference; newSize -= sizeDifference;
setSize(newSize); setSize(newSize);
at(newSize) = '\0';
} }
} }
@@ -916,7 +909,6 @@ private:
} else if (startIndex != 0) { } else if (startIndex != 0) {
size_type newSize = size() + sizeDifference; size_type newSize = size() + sizeDifference;
setSize(newSize); setSize(newSize);
at(newSize) = 0;
} }
return begin() + foundIndex; return begin() + foundIndex;

20
src/libs/utils/smallstringlayout.h
View File

@@ -124,7 +124,7 @@ struct alignas(16) StringDataLayout
template<size_type Size> template<size_type Size>
constexpr StringDataLayout(const char (&string)[Size]) noexcept constexpr StringDataLayout(const char (&string)[Size]) noexcept
{ {
if constexpr (Size <= MaximumShortStringDataAreaSize) { if constexpr (Size + 1 <= MaximumShortStringDataAreaSize) {
control = {Size - 1, false, false}; control = {Size - 1, false, false};
for (size_type i = 0; i < Size; ++i) for (size_type i = 0; i < Size; ++i)
shortString[i] = string[i]; shortString[i] = string[i];
@@ -136,14 +136,10 @@ struct alignas(16) StringDataLayout
constexpr static size_type shortStringCapacity() noexcept constexpr static size_type shortStringCapacity() noexcept
{ {
return MaximumShortStringDataAreaSize - 1; return MaximumShortStringDataAreaSize;
} }
constexpr void reset() constexpr void reset() { control = ControlBlock<MaximumShortStringDataAreaSize>(); }
{
control = ControlBlock<MaximumShortStringDataAreaSize>();
shortString[0] = '\0';
}
#pragma pack(push) #pragma pack(push)
#pragma pack(1) #pragma pack(1)
@@ -180,7 +176,7 @@ struct alignas(16) StringDataLayout<MaximumShortStringDataAreaSize,
template<size_type Size> template<size_type Size>
constexpr StringDataLayout(const char (&string)[Size]) noexcept constexpr StringDataLayout(const char (&string)[Size]) noexcept
{ {
if constexpr (Size <= MaximumShortStringDataAreaSize) { if constexpr (Size + 1 <= MaximumShortStringDataAreaSize) {
control = {Size - 1, false, false}; control = {Size - 1, false, false};
for (size_type i = 0; i < Size; ++i) for (size_type i = 0; i < Size; ++i)
shortString[i] = string[i]; shortString[i] = string[i];
@@ -204,14 +200,10 @@ struct alignas(16) StringDataLayout<MaximumShortStringDataAreaSize,
constexpr static size_type shortStringCapacity() noexcept constexpr static size_type shortStringCapacity() noexcept
{ {
return MaximumShortStringDataAreaSize - 1; return MaximumShortStringDataAreaSize;
} }
constexpr void reset() constexpr void reset() { control = ControlBlock<MaximumShortStringDataAreaSize>(); }
{
control = ControlBlock<MaximumShortStringDataAreaSize>();
shortString[0] = '\0';
}
#pragma pack(push) #pragma pack(push)
#pragma pack(1) #pragma pack(1)

38
tests/unit/unittest/smallstring-test.cpp
View File

@@ -770,7 +770,7 @@ TEST(SmallString, CapacityShortSmallString)
auto capacity = shortText.capacity(); auto capacity = shortText.capacity();
ASSERT_THAT(capacity, 30); ASSERT_THAT(capacity, 31);
} }
TEST(SmallString, CapacityLongSmallString) TEST(SmallString, CapacityLongSmallString)
@@ -807,7 +807,7 @@ TEST(SmallString, FitsInNotShortSmallStringCapacity)
{ {
SmallString text("text", 4); SmallString text("text", 4);
ASSERT_TRUE(text.fitsNotInCapacity(31)); ASSERT_TRUE(text.fitsNotInCapacity(32));
} }
TEST(SmallString, FitsInLongSmallStringCapacity) TEST(SmallString, FitsInLongSmallStringCapacity)
@@ -1327,7 +1327,7 @@ TEST(SmallString, ReserveSmallerThanShortStringCapacity)
text.reserve(2); text.reserve(2);
ASSERT_THAT(text.capacity(), AnyOf(30, 4)); ASSERT_THAT(text.capacity(), 31);
} }
TEST(SmallString, ReserveSmallerThanShortStringCapacityIsShortString) TEST(SmallString, ReserveSmallerThanShortStringCapacityIsShortString)
@@ -1354,7 +1354,7 @@ TEST(SmallString, ReserveBiggerThanShortStringCapacity)
text.reserve(10); text.reserve(10);
ASSERT_THAT(text.capacity(), AnyOf(30, 10)); ASSERT_THAT(text.capacity(), 31);
} }
TEST(SmallString, ReserveBiggerThanReference) TEST(SmallString, ReserveBiggerThanReference)
@@ -1399,7 +1399,7 @@ TEST(SmallString, ReserveSmallerThanShortSmallString)
text.reserve(10); text.reserve(10);
ASSERT_THAT(text.capacity(), 30); ASSERT_THAT(text.capacity(), 31);
} }
TEST(SmallString, ReserveBiggerThanShortSmallString) TEST(SmallString, ReserveBiggerThanShortSmallString)
@@ -1444,11 +1444,11 @@ TEST(SmallString, OptimalCapacityForSize)
SmallString text; SmallString text;
ASSERT_THAT(text.optimalCapacity(0), 0); ASSERT_THAT(text.optimalCapacity(0), 0);
ASSERT_THAT(text.optimalCapacity(30), 30); ASSERT_THAT(text.optimalCapacity(31), 31);
ASSERT_THAT(text.optimalCapacity(31), 63); ASSERT_THAT(text.optimalCapacity(32), 64);
ASSERT_THAT(text.optimalCapacity(63), 63); ASSERT_THAT(text.optimalCapacity(64), 64);
ASSERT_THAT(text.optimalCapacity(64), 127); ASSERT_THAT(text.optimalCapacity(65), 128);
ASSERT_THAT(text.optimalCapacity(128), 191); ASSERT_THAT(text.optimalCapacity(129), 192);
} }
TEST(SmallString, DataStreamData) TEST(SmallString, DataStreamData)
@@ -1792,13 +1792,6 @@ TEST(SmallString, EmptyInitializerListSize)
ASSERT_THAT(text, SizeIs(0)); ASSERT_THAT(text, SizeIs(0));
} }
TEST(SmallString, EmptyInitializerListNullTerminated)
{
auto end = SmallString::join({})[0];
ASSERT_THAT(end, '\0');
}
TEST(SmallString, InitializerListContent) TEST(SmallString, InitializerListContent)
{ {
auto text = SmallString::join({"some", " ", "text"}); auto text = SmallString::join({"some", " ", "text"});
@@ -1813,13 +1806,6 @@ TEST(SmallString, InitializerListSize)
ASSERT_THAT(text, SizeIs(9)); ASSERT_THAT(text, SizeIs(9));
} }
TEST(SmallString, InitializerListNullTerminated)
{
auto end = SmallString::join({"some", " ", "text"})[9];
ASSERT_THAT(end, '\0');
}
TEST(SmallString, NumberToString) TEST(SmallString, NumberToString)
{ {
ASSERT_THAT(SmallString::number(-0), "0"); ASSERT_THAT(SmallString::number(-0), "0");
@@ -1871,8 +1857,8 @@ TEST(SmallString, StringPlusOperatorReverseOrder)
TEST(SmallString, ShortStringCapacity) TEST(SmallString, ShortStringCapacity)
{ {
ASSERT_THAT(SmallString().shortStringCapacity(), 30); ASSERT_THAT(SmallString().shortStringCapacity(), 31);
ASSERT_THAT(PathString().shortStringCapacity(), 189); ASSERT_THAT(PathString().shortStringCapacity(), 190);
} }
TEST(SmallString, ToView) TEST(SmallString, ToView)