From db215e4bafa72ff5b5889b294098ff8c72284469 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emmanuel=20Beno=C3=AEt?= <tseeker@nocternity.net>
Date: Thu, 1 Oct 2020 13:20:50 +0200
Subject: [PATCH] Strings - Fixed appendNumeric(INT64_MIN)

Couldn't work as it was trying to store the absolute value in a signed
64-bit integer.
---
 src/Strings.cc           | 60 ++++++----------------------------------
 tests/strings-builder.cc | 50 ++++++++++++++++++++++++++++-----
 2 files changed, 52 insertions(+), 58 deletions(-)

diff --git a/src/Strings.cc b/src/Strings.cc
index 2573b17..d8037c4 100644
--- a/src/Strings.cc
+++ b/src/Strings.cc
@@ -1939,63 +1939,21 @@ T_StringBuilder& T_StringBuilder::append( T_Character character )
 T_StringBuilder& T_StringBuilder::appendNumeric( int64_t value , int base , bool useSep , T_Character sep ,
 						int sepEvery )
 {
-	assert( base >= 2 && base <= 36 );
-	assert( sepEvery > 0 );
-	assert( sep.isValid( ) );
-
 	if ( value == 0 ) {
 		return append( '0' );
 	}
-
-	const bool neg = value < 0;
-	const uint32_t size = 64 + ( useSep ? ( 64 / sepEvery ) : 0 )
-			      + ( neg ? 1 : 0 );
-	uint32_t output[ size ];
-	uint32_t len = 0 , ecap = 0;
-	int sepl = 0;
-
-	if ( neg ) {
-		value = -value;
-	}
-
-	while ( value != 0 ) {
-		const uint32_t mod = value % base;
-		output[ size - ( len + 1 ) ] = mod
-					       + ( mod < 10 ? '0' : ( 'A' - 10 ) );
-		len ++;
-		sepl ++;
-		ecap ++;
-		assert( len < size );
-
-		value = ( value - mod ) / base;
-		if ( useSep && sepl == sepEvery && value != 0 ) {
-			output[ size - ( len + 1 ) ] = sep;
-			len ++;
-			sepl = 0;
-			ecap += 4;
-			assert( len < size );
+	const bool neg{ value < 0 };
+	uint64_t absValue{ [=]{
+		if ( neg ) {
+			return uint64_t( -value );
+		} else {
+			return uint64_t( value );
 		}
-	}
-
+	}() };
 	if ( neg ) {
-		output[ size - ( len + 1 ) ] = '-';
-		len ++;
-		ecap ++;
-		assert( len < size );
+		append( '-' );
 	}
-
-	ensureCapacity( size_ + ecap );
-	ecap = 0;
-	char* ptr = data_ + size_;
-	for ( uint32_t i = size - len ; i < size ; i ++ ) {
-		const uint32_t wr = UTF8PutCodepoint( ptr , 4 , output[ i ] );
-		ptr += wr;
-		ecap += wr;
-	}
-	size_ += ecap;
-	length_ += len;
-
-	return *this;
+	return appendNumeric( absValue , base , useSep , sep , sepEvery );
 }
 
 /*----------------------------------------------------------------------------*/
diff --git a/tests/strings-builder.cc b/tests/strings-builder.cc
index e874fbb..25c292b 100644
--- a/tests/strings-builder.cc
+++ b/tests/strings-builder.cc
@@ -39,7 +39,13 @@ class StringsBuilderTest : public CppUnit::TestFixture
 		CPPUNIT_TEST( testAppendChar );
 		CPPUNIT_TEST( testAppendUnicode );
 
-		CPPUNIT_TEST( testAppendNumSigned );
+		CPPUNIT_TEST( testAppendNumSignedZeroB10 );
+		CPPUNIT_TEST( testAppendNumSignedPositiveB10 );
+		CPPUNIT_TEST( testAppendNumSignedNegativeB10 );
+		CPPUNIT_TEST( testAppendNumSignedMinB10 );
+		CPPUNIT_TEST( testAppendNumSignedMaxB10 );
+		CPPUNIT_TEST( testAppendNumSignedRest );
+
 		CPPUNIT_TEST( testAppendNumUnsigned );
 		CPPUNIT_TEST( testAppendNumFloat );
 
@@ -134,7 +140,13 @@ public:
 	void testAppendChar( );
 	void testAppendUnicode( );
 
-	void testAppendNumSigned( );
+	void testAppendNumSignedZeroB10( );
+	void testAppendNumSignedPositiveB10( );
+	void testAppendNumSignedNegativeB10( );
+	void testAppendNumSignedMinB10( );
+	void testAppendNumSignedMaxB10( );
+	void testAppendNumSignedRest( );
+
 	void testAppendNumUnsigned( );
 	void testAppendNumFloat( );
 
@@ -546,15 +558,39 @@ void StringsBuilderTest::testAppendUnicode( )
 		CPPUNIT_ASSERT( !memcmp( sbo.data( ) , t , sz ) ); \
 	}
 
-void StringsBuilderTest::testAppendNumSigned( )
+void StringsBuilderTest::testAppendNumSignedZeroB10( )
 {
 	T_StringBuilder sbo;
-	sbo.clear( ).appendNumeric( int64_t( 1 ) ); M_CHECK_( "1" );
 	sbo.clear( ).appendNumeric( int64_t( 0 ) ); M_CHECK_( "0" );
-	sbo.clear( ).appendNumeric( int64_t( -1 ) ); M_CHECK_( "-1" );
-	sbo.clear( ).appendNumeric( int64_t( 1234 ) ); M_CHECK_( "1234" );
-	sbo.clear( ).appendNumeric( int64_t( -1234 ) ); M_CHECK_( "-1234" );
+}
 
+void StringsBuilderTest::testAppendNumSignedNegativeB10( )
+{
+	T_StringBuilder sbo;
+	sbo.clear( ).appendNumeric( int64_t( -1234 ) ); M_CHECK_( "-1234" );
+}
+
+void StringsBuilderTest::testAppendNumSignedPositiveB10( )
+{
+	T_StringBuilder sbo;
+	sbo.clear( ).appendNumeric( int64_t( 1234 ) ); M_CHECK_( "1234" );
+}
+
+void StringsBuilderTest::testAppendNumSignedMinB10( )
+{
+	T_StringBuilder sbo;
+	sbo.clear( ).appendNumeric( int64_t( INT64_MIN ) ); M_CHECK_( "-9223372036854775808" );
+}
+
+void StringsBuilderTest::testAppendNumSignedMaxB10( )
+{
+	T_StringBuilder sbo;
+	sbo.clear( ).appendNumeric( int64_t( INT64_MAX ) ); M_CHECK_( "9223372036854775807" );
+}
+
+void StringsBuilderTest::testAppendNumSignedRest( )
+{
+	T_StringBuilder sbo;
 	sbo.clear( ).appendNumeric( int64_t( 129 ) , 2 );
 	M_CHECK_( "10000001" );
 	sbo.clear( ).appendNumeric( int64_t( -129 ) , 2 );