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

[Michal Simek]

On 07. 06. 20 7:36, 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)
> 
> 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
>  			*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;
> 


It is in u-boot mainline from February. Then we had to fix it in April.
In the middle of this I have seen IIC one patchset which improves hex
handling which is likely better way then this.
I am fine with reverting this patch but I would also like to see more
comments in the code to say that we shouldn't really change this.

Thanks,
Michal