From 688c14266c074ca6507884d55ca07260bc54a2b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Mon, 10 Mar 2008 14:14:44 +0000 Subject: [PATCH] * After my last change, _IsShareable() was called on the wrong string in the copy constructors, effectively turning of references completely. * Since that caused troubles (NetPositive now crashes when started), I temporarily disabled references by letting _IsShareable() always return false until the issue is resolved. * _FreePrivateData() now sets the fPrivateData member to NULL, and is also safe to be called when fPrivateData is NULL. * Removed my comment about the threading problem in _Detach() and _DetachWith() as that just couldn't happen. * _Clone() must not use memcpy() as the string pointed to by "data" might not be as long as "length". * LockBuffer() now marks a string as unshareable. * Minor cleanup. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@24345 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- headers/os/support/String.h | 5 +-- src/kits/support/String.cpp | 62 ++++++++++++++++++++++--------------- 2 files changed, 40 insertions(+), 27 deletions(-) diff --git a/headers/os/support/String.h b/headers/os/support/String.h index 2a4f565ea1..e0140d9d33 100644 --- a/headers/os/support/String.h +++ b/headers/os/support/String.h @@ -254,8 +254,9 @@ private: const char* with, int32 withLen); private: - inline int32& _ReferenceCount(); - bool _IsShareable(); + int32& _ReferenceCount(); + const int32& _ReferenceCount() const; + bool _IsShareable() const; void _FreePrivateData(); char* fPrivateData; diff --git a/src/kits/support/String.cpp b/src/kits/support/String.cpp index fcf56cbd1f..90b9fb5861 100644 --- a/src/kits/support/String.cpp +++ b/src/kits/support/String.cpp @@ -183,7 +183,7 @@ BString::BString(const BString& string) : fPrivateData(NULL) { // check if source is sharable - if so, share else clone - if (_IsShareable()) { + if (string._IsShareable()) { fPrivateData = string.fPrivateData; atomic_add(&_ReferenceCount(), 1); // string cannot go away right now @@ -283,7 +283,7 @@ BString::SetTo(const BString& string) _FreePrivateData(); // if source is sharable share, otherwise clone - if (_IsShareable()) { + if (string._IsShareable()) { fPrivateData = string.fPrivateData; atomic_add(&_ReferenceCount(), 1); // the string cannot go away right now @@ -344,8 +344,7 @@ BString::SetTo(char c, int32 count) BString& -BString::CopyInto(BString& into, int32 fromOffset, - int32 length) const +BString::CopyInto(BString& into, int32 fromOffset, int32 length) const { if (this != &into) into.SetTo(fPrivateData + fromOffset, length); @@ -501,7 +500,7 @@ BString::Insert(const char* string, int32 length, int32 position) BString& BString::Insert(const char* string, int32 fromOffset, int32 length, - int32 position) + int32 position) { if (string) Insert(string + fromOffset, length, position); @@ -519,8 +518,7 @@ BString::Insert(const BString& string, int32 position) BString& -BString::Insert(const BString& string, int32 length, - int32 position) +BString::Insert(const BString& string, int32 length, int32 position) { if ((fPrivateData != string.fPrivateData) && string.Length() > 0) Insert(string.String(), length, position); @@ -529,8 +527,8 @@ BString::Insert(const BString& string, int32 length, BString& -BString::Insert(const BString& string, int32 fromOffset, - int32 length, int32 position) +BString::Insert(const BString& string, int32 fromOffset, int32 length, + int32 position) { if ((fPrivateData != string.fPrivateData) && string.Length() > 0) Insert(string.String() + fromOffset, length, position); @@ -1078,7 +1076,7 @@ BString::ReplaceAll(char replaceThis, char withThis, int32 fromOffset) BString& BString::Replace(char replaceThis, char withThis, int32 maxReplaceCount, - int32 fromOffset) + int32 fromOffset) { fromOffset = min_clamp0(fromOffset, Length()); int32 pos = FindFirst(replaceThis, fromOffset); @@ -1142,7 +1140,7 @@ BString::ReplaceLast(const char* replaceThis, const char* withThis) BString& BString::ReplaceAll(const char* replaceThis, const char* withThis, - int32 fromOffset) + int32 fromOffset) { if (!replaceThis || !withThis || FindFirst(replaceThis) < 0) return *this; @@ -1157,7 +1155,7 @@ BString::ReplaceAll(const char* replaceThis, const char* withThis, BString& BString::Replace(const char* replaceThis, const char* withThis, - int32 maxReplaceCount, int32 fromOffset) + int32 maxReplaceCount, int32 fromOffset) { if (!replaceThis || !withThis || maxReplaceCount <= 0 || FindFirst(replaceThis) < 0) @@ -1217,7 +1215,7 @@ BString::IReplaceAll(char replaceThis, char withThis, int32 fromOffset) BString& BString::IReplace(char replaceThis, char withThis, int32 maxReplaceCount, - int32 fromOffset) + int32 fromOffset) { char tmp[2] = { replaceThis, '\0' }; fromOffset = min_clamp0(fromOffset, Length()); @@ -1282,7 +1280,7 @@ BString::IReplaceLast(const char* replaceThis, const char* withThis) BString& BString::IReplaceAll(const char* replaceThis, const char* withThis, - int32 fromOffset) + int32 fromOffset) { if (!replaceThis || !withThis || IFindFirst(replaceThis) < 0) return *this; @@ -1297,7 +1295,7 @@ BString::IReplaceAll(const char* replaceThis, const char* withThis, BString& BString::IReplace(const char* replaceThis, const char* withThis, - int32 maxReplaceCount, int32 fromOffset) + int32 maxReplaceCount, int32 fromOffset) { if (!replaceThis || !withThis || maxReplaceCount <= 0 || FindFirst(replaceThis) < 0) @@ -1402,6 +1400,8 @@ BString::LockBuffer(int32 maxLength) length += maxLength - length; _DetachWith(fPrivateData, length); + _ReferenceCount() = -1; + // mark unshareable return fPrivateData; } @@ -1418,6 +1418,8 @@ BString::UnlockBuffer(int32 length) _Realloc(length); fPrivateData[length] = '\0'; + _ReferenceCount() = 1; + // mark shareable again return *this; } @@ -1650,8 +1652,6 @@ BString::_Detach() { char* newData = fPrivateData; - // TODO: this is not thread safe! A string could be used in than one - // thread at a time (and one of those threads could create a ref) if (atomic_get(&_ReferenceCount()) > 1) { // It might be shared, and this requires special treatment newData = _Clone(fPrivateData, Length()); @@ -1714,8 +1714,10 @@ BString::_Clone(const char* data, int32 length) *(((vint32*)newData) - 2) = 1; *(((int32*)newData) - 1) = length & 0x7fffffff; - if (data && length) - memcpy(newData, data, length); + if (data && length) { + // "data" may not span over the whole length + strncpy(newData, data, length); + } return newData; } @@ -1752,8 +1754,7 @@ status_t BString::_DetachWith(const char* string, int32 length) { char* newData = NULL; - // TODO: this is not thread safe! A string could be used in than one - // thread at a time (and one of those threads could create a ref) + if (atomic_get(&_ReferenceCount()) > 1) { // we might share our data with someone else newData = _Clone(string, length); @@ -1787,17 +1788,28 @@ BString::_ReferenceCount() } -inline bool -BString::_IsShareable() +inline const int32& +BString::_ReferenceCount() const { - return fPrivateData != NULL && _ReferenceCount() >= 0; + return *(((int32 *)fPrivateData) - 2); +} + + +inline bool +BString::_IsShareable() const +{ + return false; +// return fPrivateData != NULL && _ReferenceCount() >= 0; } void BString::_FreePrivateData() { - free(fPrivateData - kPrivateDataOffset); + if (fPrivateData != NULL) { + free(fPrivateData - kPrivateDataOffset); + fPrivateData = NULL; + } }