[PATCH] Revert "lib: Improve _parse_integer_fixup_radix base 16 detection"

[Heinrich Schuchardt]

On 6/7/20 7:36 AM, Sean Anderson wrote:
> This reverts commit 0486497e2b5f4d36fa968a1a60fea358cbf70b65.
>
> The strtoul has well-defined semantics. It is defined by the C standard and
> POSIX. To quote the relevant section of the man pages,
>
>> If base is zero or 16, the string may then include a "0x" prefix, and the
>> number will be read in base 16; otherwise, a zero base is taken as 10
>> (decimal) unless the next character is '0', in which case it is taken as
>> 8 (octal).
>
> Keeping these semantics is important for several reasons. First, it is very
> surprising for standard library functions to behave differently than usual.
> Every other implementation of strtoul has different semantics than the
> implementation in U-Boot at the moment. Second, it can result in very
> surprising results from small changes. For example, changing the string
> "1f" to "20" causes the parsed value to *decrease*. Forcing use of the "0x"
> prefix to specify hexidecimal numbers is a feature, not a bug. Lastly, this
> is slightly less performant, since the entire number is parsed twice.
>
> This fixes the str_simple_strtoul test failing with
>
> test/str_ut.c:29, run_strtoul(): expect_val == val: Expected 0x44b (1099), got 0x1099ab (1087915)
> test/str_ut.c:46, str_simple_strtoul(): 0 == run_strtoul(uts, str2, 0, 1099, 4): Expected 0x0 (0), got 0x1 (1)
>

I suggest to add a Fixes line here:

Fixes: 0486497e2b5f ("lib: Improve _parse_integer_fixup_radix base 16
detection")

Cf.
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
> CC: Michal Simek <michal.simek at xilinx.com>
> CC: Shiril Tichkule <shirilt at xilinx.com>
> ---
>
>  lib/strto.c | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/lib/strto.c b/lib/strto.c
> index 3d77115d4d..c00bb5895d 100644
> --- a/lib/strto.c
> +++ b/lib/strto.c
> @@ -22,25 +22,9 @@ static const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
>  				*base = 16;
>  			else
>  				*base = 8;
> -		} else {
> -			int i = 0;
> -			char var;
> -
> +		} else

scripts/checkpatch.pl reports a problem:

CHECK: Unbalanced braces around else statement
#50: FILE: lib/strto.c:25:
+               } else

Best regards

Heinrich

>  			*base = 10;
> -
> -			do {
> -				var = tolower(s[i++]);
> -				if (var >= 'a' && var <= 'f') {
> -					*base = 16;
> -					break;
> -				}
> -
> -				if (!(var >= '0' && var <= '9'))
> -					break;
> -			} while (var);
> -		}
>  	}
> -
>  	if (*base == 16 && s[0] == '0' && tolower(s[1]) == 'x')
>  		s += 2;
>  	return s;
>