* [PATCH] cmd: part: number: remove inconsistent 0x from returned value @ 2021-03-05 7:39 Stefan Herbrechtsmeier 2021-03-05 11:52 ` Eugeniu Rosca 0 siblings, 1 reply; 9+ messages in thread From: Stefan Herbrechtsmeier @ 2021-03-05 7:39 UTC (permalink / raw) To: u-boot From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> The part number sub-command returns the hexadecimal value with a leading 0x. This is inconsistent with other values in the command and U-Boot uses hexadecimal values generally. Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@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); break; default: printf("** Unknown cmd_part_info value: %d\n", param); -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] cmd: part: number: remove inconsistent 0x from returned value 2021-03-05 7:39 [PATCH] cmd: part: number: remove inconsistent 0x from returned value Stefan Herbrechtsmeier @ 2021-03-05 11:52 ` Eugeniu Rosca 2021-03-05 14:33 ` Stefan Herbrechtsmeier 0 siblings, 1 reply; 9+ messages in thread From: Eugeniu Rosca @ 2021-03-05 11:52 UTC (permalink / raw) To: u-boot Hello Stefan, On Fri, Mar 05, 2021 at 07:39:04AM +0000, Stefan Herbrechtsmeier wrote: > From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@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. > 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. > 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. > > Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@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); -- Best regards, Eugeniu Rosca ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] cmd: part: number: remove inconsistent 0x from returned value 2021-03-05 11:52 ` Eugeniu Rosca @ 2021-03-05 14:33 ` Stefan Herbrechtsmeier 2021-03-05 14:35 ` Simon Glass 0 siblings, 1 reply; 9+ messages in thread From: Stefan Herbrechtsmeier @ 2021-03-05 14:33 UTC (permalink / raw) To: u-boot 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@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. You can avoid it only if you could mark decimal numbers and that is impossible. @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@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); > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] cmd: part: number: remove inconsistent 0x from returned value 2021-03-05 14:33 ` Stefan Herbrechtsmeier @ 2021-03-05 14:35 ` Simon Glass 2021-03-06 20:12 ` Tom Rini 0 siblings, 1 reply; 9+ messages in thread From: Simon Glass @ 2021-03-05 14:35 UTC (permalink / raw) To: u-boot Hi, On Fri, 5 Mar 2021 at 07:33, Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@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@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@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@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); > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] cmd: part: number: remove inconsistent 0x from returned value 2021-03-05 14:35 ` Simon Glass @ 2021-03-06 20:12 ` Tom Rini 2021-03-08 10:45 ` Stefan Herbrechtsmeier 0 siblings, 1 reply; 9+ messages in thread From: Tom Rini @ 2021-03-06 20:12 UTC (permalink / raw) To: u-boot 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@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@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@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@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. -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 659 bytes Desc: not available URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210306/a796e20d/attachment.sig> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] cmd: part: number: remove inconsistent 0x from returned value 2021-03-06 20:12 ` Tom Rini @ 2021-03-08 10:45 ` Stefan Herbrechtsmeier 2021-03-12 4:45 ` Simon Glass 0 siblings, 1 reply; 9+ messages in thread From: Stefan Herbrechtsmeier @ 2021-03-08 10:45 UTC (permalink / raw) To: u-boot 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@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@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@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@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. Regards Stefan ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] cmd: part: number: remove inconsistent 0x from returned value 2021-03-08 10:45 ` Stefan Herbrechtsmeier @ 2021-03-12 4:45 ` Simon Glass 2021-03-12 6:57 ` Stefan Herbrechtsmeier 0 siblings, 1 reply; 9+ messages in thread From: Simon Glass @ 2021-03-12 4:45 UTC (permalink / raw) To: u-boot Hi Stefan, On Mon, 8 Mar 2021 at 03:45, Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@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@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@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@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@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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] cmd: part: number: remove inconsistent 0x from returned value 2021-03-12 4:45 ` Simon Glass @ 2021-03-12 6:57 ` Stefan Herbrechtsmeier 2021-03-12 15:47 ` Simon Glass 0 siblings, 1 reply; 9+ messages in thread From: Stefan Herbrechtsmeier @ 2021-03-12 6:57 UTC (permalink / raw) To: u-boot 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@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@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@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@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@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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] cmd: part: number: remove inconsistent 0x from returned value 2021-03-12 6:57 ` Stefan Herbrechtsmeier @ 2021-03-12 15:47 ` Simon Glass 0 siblings, 0 replies; 9+ messages in thread From: Simon Glass @ 2021-03-12 15:47 UTC (permalink / raw) To: u-boot Hi Stefan, On Thu, 11 Mar 2021 at 23:58, Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com> wrote: > > 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@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@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@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@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@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? Well at present test uses decimal by default, unfortunately. > > > Then one day we could change the default to hex. > > Is this possible? This would be a breaking change. I'll see if I can do a patch. > > 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)`). Yes, that seems right to me. The '0' should default to hex. > > 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? I don't know of any, so a way to snprintf into an env var seems useful. Regards, Simon ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-03-12 15:47 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-05 7:39 [PATCH] cmd: part: number: remove inconsistent 0x from returned value Stefan Herbrechtsmeier 2021-03-05 11:52 ` Eugeniu Rosca 2021-03-05 14:33 ` Stefan Herbrechtsmeier 2021-03-05 14:35 ` Simon Glass 2021-03-06 20:12 ` Tom Rini 2021-03-08 10:45 ` Stefan Herbrechtsmeier 2021-03-12 4:45 ` Simon Glass 2021-03-12 6:57 ` Stefan Herbrechtsmeier 2021-03-12 15:47 ` Simon Glass
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.