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

[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.
Then one day we could change the default to hex.

Regards,
Simon