[U-Boot] [PATCH v7 02/13] lib: strto: fix metric suffix parsing in strtoul[l]

[Stefan]

On 03.09.2018 08:49, Miquel Raynal wrote:
> Hi Stefan,
> 
> Stefan Roese <sr at denx.de> wrote on Sat, 1 Sep 2018 10:48:25 +0200:
> 
>> On 31.08.2018 16:57, Miquel Raynal wrote:
>>> While 1kB or 1kiB will be parsed correctly, 1k will return the right
>>> amount, but the metric suffix will not be escaped once the char
>>> pointer updated. Fix this situation by simplifying the move of the
>>> endp pointer.
>>>> Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
>>> ---
>>>    lib/strto.c | 22 ++++++++++------------
>>>    1 file changed, 10 insertions(+), 12 deletions(-)
>>>> diff --git a/lib/strto.c b/lib/strto.c
>>> index 84f8d92d57..502a0153e7 100644
>>> --- a/lib/strto.c
>>> +++ b/lib/strto.c
>>> @@ -97,12 +97,11 @@ unsigned long ustrtoul(const char *cp, char **endp, unsigned int base)
>>>    	case 'K':
>>>    	case 'k':
>>>    		result *= 1024;
>>> -		if ((*endp)[1] == 'i') {
>>> -			if ((*endp)[2] == 'B')
>>> -				(*endp) += 3;
>>> -			else
>>> -				(*endp) += 2;
>>> -		}
>>> +		(*endp)++;
>>> +		if (**endp == 'i')
>>> +			(*endp)++;
>>> +		if (**endp == 'B')
>>> +			(*endp)++;
>>>    	}
>>>    	return result;
>>>    }
>>> @@ -122,12 +121,11 @@ unsigned long long ustrtoull(const char *cp, char **endp, unsigned int base)
>>>    	case 'K':
>>>    	case 'k':
>>>    		result *= 1024;
>>> -		if ((*endp)[1] == 'i') {
>>> -			if ((*endp)[2] == 'B')
>>> -				(*endp) += 3;
>>> -			else
>>> -				(*endp) += 2;
>>> -		}
>>> +		(*endp)++;
>>> +		if (**endp == 'i')
>>> +			(*endp)++;
>>> +		if (**endp == 'B')
>>> +			(*endp)++;
>>>    	}
>>>    	return result;
>>>    }
>>>    
>> Even though KiB is not equal to KB in general (at least in Linux
>> userspace AFAIK), lets not change this in U-Boot and always use
>> KiB and KB as a representation for 1024 (instead of 1000). So:
> 
> That's right I did not mentioned it in the commit log. I could
> update it to reflect that it is intentional to mix 'k' and 'kiB' as a
> representation of '* 12014' (already the case, but being clarified in
> the above change).

If you will send a new version of these patches, then this would be
great to clarify this usage.

Thanks,
Stefan