[PATCH] cmd: part: number: remove inconsistent 0x from returned value

[Stefan Herbrechtsmeier]

Hi Simon,

Am 12.03.2021 um 05:45 schrieb Simon Glass:
> Hi Stefan,
> 
> On Mon, 8 Mar 2021 at 03:45, Stefan Herbrechtsmeier
> <stefan.herbrechtsmeier-oss at weidmueller.com> wrote:
>>
>> Hi,
>>
>> Am 06.03.2021 um 21:12 schrieb Tom Rini:
>>> On Fri, Mar 05, 2021 at 07:35:24AM -0700, Simon Glass wrote:
>>>> Hi,
>>>>
>>>> On Fri, 5 Mar 2021 at 07:33, Stefan Herbrechtsmeier
>>>> <stefan.herbrechtsmeier-oss at weidmueller.com> wrote:
>>>>>
>>>>> Hi Eugeniu,
>>>>>
>>>>> Am 05.03.2021 um 12:52 schrieb Eugeniu Rosca:
>>>>>> Hello Stefan,
>>>>>>
>>>>>> On Fri, Mar 05, 2021 at 07:39:04AM +0000, Stefan Herbrechtsmeier wrote:
>>>>>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
>>>>>>>
>>>>>>> The part number sub-command returns the hexadecimal value with a leading
>>>>>>> 0x.
>>>>>>
>>>>>> That's to make sure that:
>>>>>>       - users have clear and unequivocal feedback that '10'
>>>>>>         returned by the command is really HEX 10, not DEC 10.
>>>>>>       - other U-Boot commands which need to take '0x10' as input
>>>>>>         will interpret it correctly, regardless of the way these
>>>>>>         other commands implement ascii-to-integer conversion.
>>>>>
>>>>> 'Almost all U-Boot commands expect numbers to be entered in hexadecimal
>>>>> input format.' [1]
>>>>>
>>>>> The filesystem commands use `simple_strtoul(.., 16)` and interpret the
>>>>> value as hexadecimal value.
>>>>>
>>>>> The 0x suggests that a 10 will be interpreted as decimal value and this
>>>>> isn't true.
>>>>>
>>>>>>> This is inconsistent with other values in the command
>>>>>>
>>>>>> It could be, but it is then better to fix the inconsistency in those
>>>>>> commands/sub-commands which add the ambiguity.
>>>>>
>>>>> Normally you are right but U-Boot by design use hexadecimal values
>>>>> without 0x. The env_set_hex functions doesn't use 0x.
>>>>>
>>>>>>> and U-Boot uses hexadecimal values generally.
>>>>>>
>>>>>> The key word is "generally", but not always. Some U-Boot commands will
>>>>>> process '10' as HEX 10 and some will process 10 as DEC 10. So, in order
>>>>>> to avoid these games, I vote for leaving the 0x in place.
>>>>
>>>> I would be very surprised if 10 means 0d10 in a partition number. I
>>>> agree that putting a 0x in these values is a dangerous precedent and
>>>> will just cause confusion. U-Boot uses hex for addresses and most
>>>> arguments
>>>>
>>>>>
>>>>> You can avoid it only if you could mark decimal numbers and that is
>>>>> impossible.
>>>>
>>>> 0d10 is available. People are not used to it though.
>>>>
>>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>>
>>>>>
>>>>> @Tom: Does U-Boot still expect numbers to be hexadecimal values?
>>>>>
>>>>> [1] https://www.denx.de/wiki/DULG/UBootCommandLineInterface
>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>>     cmd/part.c | 2 +-
>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/cmd/part.c b/cmd/part.c
>>>>>>> index 3395c17b89..56e1852c66 100644
>>>>>>> --- a/cmd/part.c
>>>>>>> +++ b/cmd/part.c
>>>>>>> @@ -152,7 +152,7 @@ static int do_part_info(int argc, char *const argv[], enum cmd_part_info param)
>>>>>>>                snprintf(buf, sizeof(buf), LBAF, info.size);
>>>>>>>                break;
>>>>>>>        case CMD_PART_INFO_NUMBER:
>>>>>>> -            snprintf(buf, sizeof(buf), "0x%x", part);
>>>>>>> +            snprintf(buf, sizeof(buf), "%x", part);
>>>>>>
>>>
>>> I am not a fan of this change as well, especially having spent time on
>>> some platforms that have literally 20+ partition entries.  Being clear
>>> here that this is a hex value is important.
>>
>> But isn't it confusing to use a 0x for a value which is treated as
>> hexadecimal value by the commands independent of the 0x. The 0x results
>> in the assumption that the partition is a decimal value without the 0x.
>>
>> What is the correct way to convert a hexadecimal value into a decimal
>> value on the shell? I need a value without prefix (decimal value) to
>> pass it to the root parameter of the bootargs.
>>
>> At the moment the different default numeral systems of common functions
>> like load and test are very irritating. The load command uses
>> hexadecimal value and doesn't support decimal value. The test command
>> uses decimal value by default but supports hexadecimal values with a
>> prefix. This means that the returned variable filesize of the command
>> load couldn't be checked by test and the user have to use the itest command.
> 
> Let's use hex where possible. I am not sure what to do with the test
> command, nor how often we use 'test' with addresses, but I think
> adding an option to 'test' to make it use hex or dec would be good.

But what is the difference to test vs itest?

> Then one day we could change the default to hex.

Is this possible? This would be a breaking change.

I think the main problem is the unclear default numeral system. It would 
be much easy if every number without a prefix have the same numeral 
system. This would mean that a `simple_strtoul(.., 0)` would fall back 
to the default numeral system if the value hasn't any prefix (0x or 0d). 
At the moment `simple_strtoul(.., 0)` fall back to decimal and many 
commands use different numeral systems (`simple_strtoul(.., 10)` or 
`simple_strtoul(.., 16)`).

Independent of this problem I need a command to convert a value to 
decimal without any prefix. Can you recommend any command or should I 
add a printf command?

Regards
   Stefan