support/String: Revert 6c67c7d63 to fix double-free

6c67c7d63 was attempting to fix a leak caught by a static analysis
tool, but it actually just introduced a double-free bug. Running
`UnitTester BString` will result in a crash.

The original code was correct because, in the event that realloc()
fails in BString::_Resize(), the value of fPrivateData is still
retained. It will be freed by the destructor of BString only if
fPrivateData is not shared by another BString instance, since BStrings
are copy-on-write.

Note that while the change in 6c67c7d63 caused tests to fail, that
doesn't mean those tests are ideal. They only trigger
BString::_Resize() to fail because they depend on implementation
details of hoard2 which limits allocations via malloc() to
1GB. Most malloc() implementations will allow allocations of arbitrary
sizes using anonymous mappings (mmap on Linux, or create_area() in
Haiku). This is a much bigger change, so for now I'm just adding some
comments so that we can revisit these tests if we make a change to the
allocator.

Change-Id: I208c1c7a76b6b4409d237b911c62bb3198e49dab
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2060
Reviewed-by: Stephan Aßmus <superstippi@gmx.de>

src/kits/support/String.cpp

@@ -2360,14 +2360,9 @@ BString::_Resize(int32 length)
 	if (length < 0)
 		length = 0;
 
-	char* newData = (char*)realloc(data, length + kPrivateDataOffset + 1);
-	if (newData == NULL) {
-		free(data);
-		data = NULL;
+	data = (char*)realloc(data, length + kPrivateDataOffset + 1);
+	if (data == NULL)
 		return NULL;
-	} else {
-		data = newData;
-	}
 
 	data += kPrivateDataOffset;
 

src/tests/kits/support/bstring/StringAppendTest.cpp

@@ -116,7 +116,14 @@ StringAppendTest::PerformTest(void)
 	CPPUNIT_ASSERT(strcmp(str1->String(), "BaseCCCCC") == 0);
 	delete str1;
 
-	
+	// TODO: The following test cases only work with hoard2, which will not
+	// allow allocations via malloc() larger than the largest size-class
+	// (see threadHeap::malloc(size_t). Other malloc implementations like
+	// rpmalloc will allow arbitrarily large allocations via create_area().
+	//
+	// This test should be made more robust by breaking the dependency on
+	// the allocator to simulate failures in another way. This may require
+	// a tricky build configuration to avoid breaking the ABI of BString.
 #ifndef TEST_R5
 	const int32 OUT_OF_MEM_VAL = 2 * 1000 * 1000 * 1000;
 	// Append(char, int32) with excessive length:

src/tests/kits/support/bstring/StringAssignTest.cpp

@@ -99,6 +99,14 @@ StringAssignTest::PerformTest(void)
 	delete str;
 
 #ifndef TEST_R5
+	// TODO: The following test cases only work with hoard2, which will not
+	// allow allocations via malloc() larger than the largest size-class
+	// (see threadHeap::malloc(size_t). Other malloc implementations like
+	// rpmalloc will allow arbitrarily large allocations via create_area().
+	//
+	// This test should be made more robust by breaking the dependency on
+	// the allocator to simulate failures in another way. This may require
+	// a tricky build configuration to avoid breaking the ABI of BString.
 	const int32 OUT_OF_MEM_VAL = 2 * 1000 * 1000 * 1000;
 	// SetTo(char, int32) with excessive length:
 	NextSubTest();