Fix strtol(3) quality.

libc/stdlib/strtol.cpp

@@ -1,6 +1,6 @@
 /*******************************************************************************
 
-    Copyright(C) Jonas 'Sortie' Termansen 2011, 2013.
+    Copyright(C) Jonas 'Sortie' Termansen 2011, 2013, 2014.
 
     This file is part of the Sortix C Library.
 
@@ -73,6 +73,10 @@ static bool would_multiplication_overflow(T_INT a, T_INT b)
 	if ( !a || !b )
 		return false;
 
+	// Trivial cases.
+	if ( a == 1 || b == 1 )
+		return false;
+
 	// Check if we have a 64-bit integer that it is large enough.
 	if ( sizeof(T_INT)*2 <= sizeof(int64_t) )
 	{
@@ -111,7 +115,7 @@ static bool would_multiplication_overflow(T_INT a, T_INT b)
 		T_UNSIGNED_INT b_abs = b < 0 ? - (T_UNSIGNED_INT) b : b;
 		T_UNSIGNED_INT min_abs = - (T_UNSIGNED_INT) T_INT_MIN;
 		T_UNSIGNED_INT max_abs = T_INT_MAX;
-		T_UNSIGNED_INT limit_pos = (0 <= a && 0 <= b) || ( a < 0 && b < 0 );
+		T_UNSIGNED_INT limit_pos = (0 <= a && 0 <= b) || (a < 0 && b < 0);
 		T_UNSIGNED_INT limit = limit_pos ? max_abs : min_abs;
 		T_UNSIGNED_INT max_b = a_abs / limit;
 		return max_b < b_abs;
@@ -120,24 +124,23 @@ static bool would_multiplication_overflow(T_INT a, T_INT b)
 
 extern "C"
 STRTOL_INT STRTOL(const STRTOL_CHAR* restrict str,
-                  STRTOL_CHAR** restrict endptr,
+                  STRTOL_CHAR** restrict end_ptr,
                   int base)
 {
-	const STRTOL_CHAR* origstr = str;
+	// Reject bad bases.
-	int origbase = base;
+	if ( base < 0 || base == 1 || 36 < base )
+	{
+		if ( end_ptr )
+			*end_ptr = (STRTOL_CHAR*) str;
+		return errno = EINVAL, 0;
+	}
+
+	const STRTOL_CHAR* original_str = str;
 
 	// Skip any leading white space.
 	while ( STRTOL_ISSPACE(*str) )
 		str++;
 
-	// Reject bad bases.
-	if ( base < 0 || 36 < base )
-	{
-		if ( endptr )
-			*endptr = (STRTOL_CHAR*) str;
-		return errno = EINVAL, 0;
-	}
-
 	bool negative = false;
 	STRTOL_CHAR c = *str;
 
@@ -147,43 +150,52 @@ STRTOL_INT STRTOL(const STRTOL_CHAR* restrict str,
 	else if ( c == STRTOL_L('+') )
 		str++, negative = false;
 
-	// Autodetect base 8 or base 16.
+	bool actually_negative = !STRTOL_INT_IS_UNSIGNED && negative;
-	if ( !base && str[0] == STRTOL_L('0') )
+
+	// Autodetect base if requested.
+	if ( base == 0 )
 	{
-		if ( (str[1] == STRTOL_L('x') || str[1] == STRTOL_L('X')) &&
+		if ( str[0] == STRTOL_L('0') &&
-		     (str[2] && debase(str[2]) < 16) )
+		     (str[1] == STRTOL_L('x') || str[1] == STRTOL_L('X')) &&
+		     (0 <= debase(str[2]) && debase(str[2]) < 16) )
 			str += 2, base = 16;
-		else if ( 0 <= debase(str[1]) )
+		else if ( str[0] == STRTOL_L('0') &&
+		          0 <= debase(str[1]) && debase(str[1]) < 8 )
 			str++, base = 8;
+		else
+			base = 10;
 	}
 
-	// Default to base 10.
-	if ( !base )
-		base = 10;
-
 	// Skip the leading '0x' prefix in base 16 for hexadecimal integers.
-	if ( origbase == 16 &&
+	else if ( base == 16 )
-	     str[0] == STRTOL_L('0') &&
+	{
+		if ( str[0] == STRTOL_L('0') &&
 		     (str[1] == STRTOL_L('x') || str[1] == STRTOL_L('X')) &&
 		     (0 <= debase(str[2]) && debase(str[2]) < 16) )
 			str += 2;
+	}
 
 	// Determine what value will be returned on overflow/underflow.
-	STRTOL_INT overflow_value = negative && !STRTOL_INT_IS_UNSIGNED ?
+	STRTOL_INT overflow_value =
-	                            STRTOL_INT_MIN :
+		actually_negative ? STRTOL_INT_MIN : STRTOL_INT_MAX;
-	                            STRTOL_INT_MAX;
 
 	// Convert a single character at a time.
 	STRTOL_INT result = 0;
-	size_t numconvertedchars = 0;
+	size_t num_converted_chars = 0;
 	bool overflow_occured = false;
 	while ( (c = *str ) )
 	{
-		// Stop if we encounterd a character that doesn't fit in this base.
+		// Stop if we encounter a character that doesn't fit in this base.
 		int val = debase(c);
 		if ( val < 0 || base <= val )
 			break;
 
+		str++;
+		num_converted_chars++;
+
+		if ( overflow_occured )
+			continue;
+
 		// Attempt to multiply the accumulator with the current base.
 		if ( would_multiplication_overflow<STRTOL_INT,
 		                                   STRTOL_INT_IS_UNSIGNED,
@@ -191,47 +203,72 @@ STRTOL_INT STRTOL(const STRTOL_CHAR* restrict str,
 		                                   STRTOL_INT_MIN,
 		                                   STRTOL_INT_MAX>
 		                                  (result, (STRTOL_INT) base) )
-			overflow_occured = true, result = overflow_value;
+		{
+			overflow_occured = true;
+			result = overflow_value;
+			errno = ERANGE;
+			continue;
+		}
 		else
 		{
 			STRTOL_INT new_result = result * (STRTOL_INT) base;
-			assert( negative || result <= new_result);
+			assert( actually_negative || result <= new_result);
-			assert(!negative || result >= new_result);
+			assert(!actually_negative || result >= new_result);
 			result = new_result;
 		}
 
-		// Attempt to add the latest digit to the accumulator (positive).
+		// Nothing needs to be added if we are encountered a zero digit.
-		if ( (STRTOL_INT_IS_UNSIGNED || !negative) &&
+		if ( val == 0 )
-		     (STRTOL_INT) val <= (STRTOL_INT) (STRTOL_INT_MAX - result) )
+		{
-			result += (STRTOL_INT) val;
-
-		// Attempt to subtract the latest digit to the accumulator (negative).
-		else if ( (!STRTOL_INT_IS_UNSIGNED && negative) &&
-		          (STRTOL_UNSIGNED_INT) val < ((STRTOL_UNSIGNED_INT) result - (STRTOL_UNSIGNED_INT) STRTOL_INT_MIN) )
-			result -= (STRTOL_INT) val;
-
-		// Handle the case where the addition/subtract would overflow/underflow.
-		else
-			overflow_occured = true, result = overflow_value;
-
-		str++;
-		numconvertedchars++;
 		}
 
-	// If no characters were successfully converted, rewind to the start, also
+		// Attempt to add the latest digit to the accumulator (positive).
-	// rewinding past skipped whitespace and sign characters and such.
+		else if ( !actually_negative &&
-	if ( !numconvertedchars )
+		          (STRTOL_INT) val <= (STRTOL_INT) (STRTOL_INT_MAX - result) )
-		str = origstr, result = 0;
+		{
+			result += (STRTOL_INT) val;
+		}
+
+
+		// Attempt to subtract the latest digit to the accumulator (negative).
+		else if ( actually_negative &&
+		          (STRTOL_UNSIGNED_INT) val <
+		          ((STRTOL_UNSIGNED_INT) result -
+		           (STRTOL_UNSIGNED_INT) STRTOL_INT_MIN) )
+		{
+			result -= (STRTOL_INT) val;
+		}
+
+		// Otherwise, the addition/subtract would overflow/underflow.
+		else
+		{
+			overflow_occured = true;
+			result = overflow_value;
+			errno = ERANGE;
+			continue;
+		}
+	}
+
+	// If no characters were successfully converted, rewind to the start.
+	if ( !num_converted_chars )
+	{
+		errno = EINVAL;
+		str = original_str;
+	}
 
 	// Let the caller know where we got to.
-	if ( endptr )
+	if ( end_ptr )
-		*endptr = (STRTOL_CHAR*) str;
+		*end_ptr = (STRTOL_CHAR*) str;
 
 	// Handle the special case where we are creating an unsigned integer and the
-	// string was negative and non-zero and no overflow occured, then we treat
+	// string was negative. The result is the negation assuming no overflow.
-	// it as (the maximum value+1) minus (the negative string as integer).
+	if ( STRTOL_INT_IS_UNSIGNED && negative )
-	if ( STRTOL_INT_IS_UNSIGNED && negative && result && !overflow_occured )
+	{
-		result = STRTOL_INT_MAX - (result-1);
+		if ( overflow_occured )
+			result = STRTOL_INT_MAX;
+		else
+			result = -result;
+	}
 
 	return result;
 }