From 530f89aa6d0a551aa8ab4a3af16d88186f04878c Mon Sep 17 00:00:00 2001
From: Niels Sascha Reedijk <niels.reedijk@gmail.com>
Date: Thu, 9 Sep 2021 22:28:10 +0100
Subject: [PATCH] BString: rewrite cleanup methods

This removes the use of the destructor in the move assignment operator, as it
may rely on undefined behaviour from the compiler. Additionally, some duplicate
logic to dereference and free a shared string has been unified under
_ReleasePrivateData().

Change-Id: Ie9f51d598c734f83cd0fba49b651315c6e9c8aac
Reviewed-on: https://review.haiku-os.org/c/haiku/+/4440
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
Tested-by: Commit checker robot <no-reply+buildbot@haiku-os.org>
---
 headers/os/support/String.h |  1 +
 src/kits/support/String.cpp | 35 ++++++++++++++---------------------
 2 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/headers/os/support/String.h b/headers/os/support/String.h
index 2b63510448..62ab0f3db6 100644
--- a/headers/os/support/String.h
+++ b/headers/os/support/String.h
@@ -423,6 +423,7 @@ private:
 			const int32& 	_ReferenceCount() const;
 			bool			_IsShareable() const;
 			void			_FreePrivateData();
+			void			_ReleasePrivateData();
 
 			char*			fPrivateData;
 };
diff --git a/src/kits/support/String.cpp b/src/kits/support/String.cpp
index 843f0994ea..f684e641f7 100644
--- a/src/kits/support/String.cpp
+++ b/src/kits/support/String.cpp
@@ -204,8 +204,7 @@ BString::BString(BString&& string)
 
 BString::~BString()
 {
-	if (!_IsShareable() || atomic_add(&_ReferenceCount(), -1) == 1)
-		_FreePrivateData();
+	_ReleasePrivateData();
 }
 
 
@@ -277,8 +276,7 @@ BString&
 BString::operator=(BString&& string)
 {
 	if (this != &string) {
-		this->~BString();
-			// free up any resources allocated by the current contents
+		_ReleasePrivateData();
 		fPrivateData = string.fPrivateData;
 		string.fPrivateData = NULL;
 	}
@@ -309,15 +307,7 @@ BString::SetTo(const BString& string)
 	if (fPrivateData == string.fPrivateData)
 		return *this;
 
-	bool freeData = true;
-
-	if (_IsShareable() && atomic_add(&_ReferenceCount(), -1) > 1) {
-		// there is still someone who shares our data
-		freeData = false;
-	}
-
-	if (freeData)
-		_FreePrivateData();
+	_ReleasePrivateData();
 
 	// if source is sharable share, otherwise clone
 	if (string._IsShareable()) {
@@ -2293,10 +2283,7 @@ BString::_MakeWritable()
 	if (atomic_get(&_ReferenceCount()) > 1) {
 		// It might be shared, and this requires special treatment
 		char* newData = _Clone(fPrivateData, Length());
-		if (atomic_add(&_ReferenceCount(), -1) == 1) {
-			// someone else left, we were the last owner
-			_FreePrivateData();
-		}
+		_ReleasePrivateData();
 		if (newData == NULL)
 			return B_NO_MEMORY;
 
@@ -2328,10 +2315,7 @@ BString::_MakeWritable(int32 length, bool copy)
 		if (newData == NULL)
 			return B_NO_MEMORY;
 
-		if (atomic_add(&_ReferenceCount(), -1) == 1) {
-			// someone else left, we were the last owner
-			_FreePrivateData();
-		}
+		_ReleasePrivateData();
 	} else {
 		// we don't share our data with someone else
 		newData = _Resize(length);
@@ -2469,6 +2453,15 @@ BString::_FreePrivateData()
 }
 
 
+void
+BString::_ReleasePrivateData()
+{
+	if (!_IsShareable() || atomic_add(&_ReferenceCount(), -1) == 1)
+		_FreePrivateData();
+	fPrivateData = NULL;
+}
+
+
 bool
 BString::_DoAppend(const char* string, int32 length)
 {