[U-Boot] [PATCH] cmd_nvedit.c: setenv_hex must prefix hex with '0x'

[U-Boot] [PATCH] cmd_nvedit.c: setenv_hex must prefix hex with '0x'

[Tom Rini]

setenv_hex is only called with hex values, but does not prefix the
strings with '0x', as in general U-Boot assumes hex values not decimal
values, and this saves space in the environment.  However, some
functions such as 'load' take some values that are most easily described
in hex (load address) and decimal (size, offset within a file).

This can lead to the situation where, for example, spl export is run,
which leads to a call of setenv_hex of the fdtaddr, which will be re-set
in the environment.  Then 'saveenv' may be run (after updating other
parts of the environment for falcon mode), causing an invalid for 'load'
fdtaddr to be saved to the environment and leading to future boots to
fail if using 'load' to read the fdt file.

Cc: Wolfgang Denk <wd@denx.de>
Signed-off-by: Tom Rini <trini@ti.com>
---
 common/cmd_nvedit.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 778dca5..4cac794 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -321,7 +321,7 @@ int setenv_hex(const char *varname, ulong value)
 {
 	char str[17];
 
-	sprintf(str, "%lx", value);
+	sprintf(str, "0x%lx", value);
 	return setenv(varname, str);
 }
 
-- 
1.7.9.5

[U-Boot] [PATCH] cmd_nvedit.c: setenv_hex must prefix hex with '0x'

[Wolfgang Denk]

Dear Tom Rini,

In message <1380901758-30360-1-git-send-email-trini@ti.com> you wrote:
> setenv_hex is only called with hex values, but does not prefix the
> strings with '0x', as in general U-Boot assumes hex values not decimal
> values, and this saves space in the environment.  However, some
> functions such as 'load' take some values that are most easily described
> in hex (load address) and decimal (size, offset within a file).
> 
> This can lead to the situation where, for example, spl export is run,
> which leads to a call of setenv_hex of the fdtaddr, which will be re-set
> in the environment.  Then 'saveenv' may be run (after updating other
> parts of the environment for falcon mode), causing an invalid for 'load'
> fdtaddr to be saved to the environment and leading to future boots to
> fail if using 'load' to read the fdt file.

I think this is not a correct way to fix the issues at hand.

All U-Boot (with the single unfortunate exception of the "sleep"
command) defaults to hex input - that's what's documented, and what
people have always been using for many, many years.  Applying your
patch means that we modify just a single use case, while setting the
same environment variables from the command line (without the leading
0x) would still trigger a problem.

I understand that the actual cause of these issues is commit 3f83c87
"fs: fix number base behaviour change in fatload/ext*load".  Reviewing
this patch I think that it should have never been applied.  The fact
that  "load" and "fatload" / "ext*load" behave differently are reason
enough to reject this patch.  "load" should just behave like every
other command, and default to hex input.

I think we should NAK your patch, and suggest to fix the problem by
reverting commit 3f83c87 and making "load" default to hex input mode.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Landing: a controlled mid-air collision with a planet.

[U-Boot] [PATCH] cmd_nvedit.c: setenv_hex must prefix hex with '0x'

[Stephen Warren]

On 10/04/2013 03:35 PM, Wolfgang Denk wrote:
> Dear Tom Rini,
> 
> In message <1380901758-30360-1-git-send-email-trini@ti.com> you wrote:
>> setenv_hex is only called with hex values, but does not prefix the
>> strings with '0x', as in general U-Boot assumes hex values not decimal
>> values, and this saves space in the environment.  However, some
>> functions such as 'load' take some values that are most easily described
>> in hex (load address) and decimal (size, offset within a file).
>>
>> This can lead to the situation where, for example, spl export is run,
>> which leads to a call of setenv_hex of the fdtaddr, which will be re-set
>> in the environment.  Then 'saveenv' may be run (after updating other
>> parts of the environment for falcon mode), causing an invalid for 'load'
>> fdtaddr to be saved to the environment and leading to future boots to
>> fail if using 'load' to read the fdt file.
> 
> I think this is not a correct way to fix the issues at hand.
> 
> All U-Boot (with the single unfortunate exception of the "sleep"
> command) defaults to hex input - that's what's documented, and what
> people have always been using for many, many years.  Applying your
> patch means that we modify just a single use case, while setting the
> same environment variables from the command line (without the leading
> 0x) would still trigger a problem.
> 
> I understand that the actual cause of these issues is commit 3f83c87
> "fs: fix number base behaviour change in fatload/ext*load".  Reviewing
> this patch I think that it should have never been applied.  The fact
> that  "load" and "fatload" / "ext*load" behave differently are reason
> enough to reject this patch.  "load" should just behave like every
> other command, and default to hex input.
> 
> I think we should NAK your patch, and suggest to fix the problem by
> reverting commit 3f83c87 and making "load" default to hex input mode.

Reverting 3f83c87 would do the opposite of what you want; it'd make
extload/fatload require 0x prefixes instead of assuming hex. Perhaps
what you want is a tweak to that patch so that the generic load/ls
commands always expect a hex value, rather than requiring the 0x prefix?

[U-Boot] [PATCH] cmd_nvedit.c: setenv_hex must prefix hex with '0x'

[Wolfgang Denk]

Dear Stephen,

In message <524F376C.7070703@wwwdotorg.org> you wrote:
>
> > I think we should NAK your patch, and suggest to fix the problem by
> > reverting commit 3f83c87 and making "load" default to hex input mode.
> 
> Reverting 3f83c87 would do the opposite of what you want; it'd make
> extload/fatload require 0x prefixes instead of assuming hex. Perhaps
> what you want is a tweak to that patch so that the generic load/ls
> commands always expect a hex value, rather than requiring the 0x prefix?

Well, that should be the result, yes.

You mean the extload/fatload commands have been broken before that?
OK, eventually the bug was introduced before that.  BUt in any case
it's a bug, and should be fixed.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Quantum Mechanics is God's version of "Trust me."

[U-Boot] [PATCH] cmd_nvedit.c: setenv_hex must prefix hex with '0x'

[Stephen Warren]

On 10/04/2013 04:12 PM, Wolfgang Denk wrote:
> Dear Stephen,
> 
> In message <524F376C.7070703@wwwdotorg.org> you wrote:
>>
>>> I think we should NAK your patch, and suggest to fix the problem by
>>> reverting commit 3f83c87 and making "load" default to hex input mode.
>>
>> Reverting 3f83c87 would do the opposite of what you want; it'd make
>> extload/fatload require 0x prefixes instead of assuming hex. Perhaps
>> what you want is a tweak to that patch so that the generic load/ls
>> commands always expect a hex value, rather than requiring the 0x prefix?
> 
> Well, that should be the result, yes.
> 
> You mean the extload/fatload commands have been broken before that?
> OK, eventually the bug was introduced before that.  BUt in any case
> it's a bug, and should be fixed.

extload/fatload were broken between the following two commits:

3f83c87 fs: fix number base behaviour change in fatload/ext*load
...
045fa1e fs: add filesystem switch libary, implement ls and fsload commands

(i.e. for about 50 commits)

The generic load command has been "broken" (by design...) since it was
introduced. I suppose you can change the behaviour if you want; anyone
writing "0x..." for their values presumably won't be affected, and if
people really do assume all values in U-Boot are in hex, presumably
nobody currently relies upon using non-prefixed values with the generic
load command, since it doesn't work like that right now.

[U-Boot] [PATCH] Fix number base handling of "load" command

[Wolfgang Denk]

A documented, almost all U-Boot commands expect numbers to be entered
in hexadecimal input format. (Exception: for historical reasons, the
"sleep" command takes its argument in decimal input format.)

This rule was broken for the "load" command; for details please see
especially commits 045fa1e "fs: add filesystem switch libary,
implement ls and fsload commands" and 3f83c87 "fs: fix number base
behaviour change in fatload/ext*load".  In the result, the load
command would always require an explicit "0x" prefix for regular
(i. e. base 16 formatted) input.

Change this to use the standard notation of base 16 input format.
While strictly speaking this is a change of the user interface, we
hope that it will not cause trouble.  Stephen Warren comments (see
[1]):

        I suppose you can change the behaviour if you want; anyone
        writing "0x..." for their values presumably won't be
        affected, and if people really do assume all values in U-Boot
        are in hex, presumably nobody currently relies upon using
        non-prefixed values with the generic load command, since it
        doesn't work like that right now.

[1] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/171172

Signed-off-by: Wolfgang Denk <wd@denx.de>
cc: Tom Rini <trini@ti.com>, u-boot at lists.denx.de
cc: Stephen Warren <swarren@nvidia.com>
---
The patch has been build-tested (running MAKEALL for Power and ARM
architectures) only.


 common/cmd_ext2.c |  5 ++---
 common/cmd_ext4.c |  5 ++---
 common/cmd_fat.c  |  5 ++---
 common/cmd_fs.c   |  6 ++----
 fs/fs.c           | 16 ++++++++--------
 include/fs.h      |  4 ++--
 6 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/common/cmd_ext2.c b/common/cmd_ext2.c
index 1731919..5a4bcc1 100644
--- a/common/cmd_ext2.c
+++ b/common/cmd_ext2.c
@@ -32,7 +32,7 @@ int do_ext2ls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
  */
 int do_ext2load (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	return do_load(cmdtp, flag, argc, argv, FS_TYPE_EXT, 16);
+	return do_load(cmdtp, flag, argc, argv, FS_TYPE_EXT);
 }
 
 U_BOOT_CMD(
@@ -47,6 +47,5 @@ U_BOOT_CMD(
 	"load binary file from a Ext2 filesystem",
 	"<interface> <dev[:part]> [addr] [filename] [bytes]\n"
 	"    - load binary file 'filename' from 'dev' on 'interface'\n"
-	"      to address 'addr' from ext2 filesystem.\n"
-	"      All numeric parameters are assumed to be hex."
+	"      to address 'addr' from ext2 filesystem."
 );
diff --git a/common/cmd_ext4.c b/common/cmd_ext4.c
index 4a27cd9..8289d25 100644
--- a/common/cmd_ext4.c
+++ b/common/cmd_ext4.c
@@ -45,7 +45,7 @@
 int do_ext4_load(cmd_tbl_t *cmdtp, int flag, int argc,
 						char *const argv[])
 {
-	return do_load(cmdtp, flag, argc, argv, FS_TYPE_EXT, 16);
+	return do_load(cmdtp, flag, argc, argv, FS_TYPE_EXT);
 }
 
 int do_ext4_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
@@ -122,5 +122,4 @@ U_BOOT_CMD(ext4load, 6, 0, do_ext4_load,
 	   "load binary file from a Ext4 filesystem",
 	   "<interface> <dev[:part]> [addr] [filename] [bytes]\n"
 	   "    - load binary file 'filename' from 'dev' on 'interface'\n"
-	   "      to address 'addr' from ext4 filesystem.\n"
-	   "      All numeric parameters are assumed to be hex.");
+	   "      to address 'addr' from ext4 filesystem");
diff --git a/common/cmd_fat.c b/common/cmd_fat.c
index f6d7aff..a12d8fa 100644
--- a/common/cmd_fat.c
+++ b/common/cmd_fat.c
@@ -19,7 +19,7 @@
 
 int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	return do_load(cmdtp, flag, argc, argv, FS_TYPE_FAT, 16);
+	return do_load(cmdtp, flag, argc, argv, FS_TYPE_FAT);
 }
 
 
@@ -35,8 +35,7 @@ U_BOOT_CMD(
 	"      the load stops on end of file.\n"
 	"      If either 'pos' or 'bytes' are not aligned to\n"
 	"      ARCH_DMA_MINALIGN then a misaligned buffer warning will\n"
-	"      be printed and performance will suffer for the load.\n"
-	"      All numeric parameters are assumed to be hex."
+	"      be printed and performance will suffer for the load."
 );
 
 static int do_fat_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
diff --git a/common/cmd_fs.c b/common/cmd_fs.c
index a681d03..91a205a 100644
--- a/common/cmd_fs.c
+++ b/common/cmd_fs.c
@@ -22,7 +22,7 @@
 
 int do_load_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	return do_load(cmdtp, flag, argc, argv, FS_TYPE_ANY, 0);
+	return do_load(cmdtp, flag, argc, argv, FS_TYPE_ANY);
 }
 
 U_BOOT_CMD(
@@ -34,9 +34,7 @@ U_BOOT_CMD(
 	"      'bytes' gives the size to load in bytes.\n"
 	"      If 'bytes' is 0 or omitted, the file is read until the end.\n"
 	"      'pos' gives the file byte position to start reading from.\n"
-	"      If 'pos' is 0 or omitted, the file is read from the start.\n"
-	"      All numeric parameters are assumed to be decimal,\n"
-	"      unless specified otherwise using a leading \"0x\"."
+	"      If 'pos' is 0 or omitted, the file is read from the start."
 );
 
 int do_ls_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
diff --git a/fs/fs.c b/fs/fs.c
index 99e516a..be1855d 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -231,7 +231,7 @@ int fs_write(const char *filename, ulong addr, int offset, int len)
 }
 
 int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
-		int fstype, int cmdline_base)
+		int fstype)
 {
 	unsigned long addr;
 	const char *addr_str;
@@ -250,7 +250,7 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		return 1;
 
 	if (argc >= 4) {
-		addr = simple_strtoul(argv[3], NULL, cmdline_base);
+		addr = simple_strtoul(argv[3], NULL, 16);
 	} else {
 		addr_str = getenv("loadaddr");
 		if (addr_str != NULL)
@@ -268,11 +268,11 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		}
 	}
 	if (argc >= 6)
-		bytes = simple_strtoul(argv[5], NULL, cmdline_base);
+		bytes = simple_strtoul(argv[5], NULL, 16);
 	else
 		bytes = 0;
 	if (argc >= 7)
-		pos = simple_strtoul(argv[6], NULL, cmdline_base);
+		pos = simple_strtoul(argv[6], NULL, 16);
 	else
 		pos = 0;
 
@@ -313,7 +313,7 @@ int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 }
 
 int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
-		int fstype, int cmdline_base)
+		int fstype)
 {
 	unsigned long addr;
 	const char *filename;
@@ -329,10 +329,10 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		return 1;
 
 	filename = argv[3];
-	addr = simple_strtoul(argv[4], NULL, cmdline_base);
-	bytes = simple_strtoul(argv[5], NULL, cmdline_base);
+	addr = simple_strtoul(argv[4], NULL, 16);
+	bytes = simple_strtoul(argv[5], NULL, 16);
 	if (argc >= 7)
-		pos = simple_strtoul(argv[6], NULL, cmdline_base);
+		pos = simple_strtoul(argv[6], NULL, 16);
 	else
 		pos = 0;
 
diff --git a/include/fs.h b/include/fs.h
index c837bae..7d9403e 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -59,10 +59,10 @@ int fs_read(const char *filename, ulong addr, int offset, int len);
  * to a specific filesystem type via the fstype parameter.
  */
 int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
-		int fstype, int cmdline_base);
+		int fstype);
 int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		int fstype);
 int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
-		int fstype, int cmdline_base);
+		int fstype);
 
 #endif /* _FS_H */
-- 
1.8.3.1

[U-Boot] [PATCH] Fix number base handling of "load" command

[Stephen Warren]

On 10/05/2013 01:07 PM, Wolfgang Denk wrote:
> A documented, almost all U-Boot commands expect numbers to be entered

s/A/As/ (btw, where?)

> in hexadecimal input format. (Exception: for historical reasons, the
> "sleep" command takes its argument in decimal input format.)
> 
> This rule was broken for the "load" command; for details please see
> especially commits 045fa1e "fs: add filesystem switch libary,
> implement ls and fsload commands" and 3f83c87 "fs: fix number base
> behaviour change in fatload/ext*load".  In the result, the load
> command would always require an explicit "0x" prefix for regular
> (i. e. base 16 formatted) input.
> 
> Change this to use the standard notation of base 16 input format.
> While strictly speaking this is a change of the user interface, we
> hope that it will not cause trouble.  Stephen Warren comments (see
> [1]):
> 
>         I suppose you can change the behaviour if you want; anyone
>         writing "0x..." for their values presumably won't be
>         affected, and if people really do assume all values in U-Boot
>         are in hex, presumably nobody currently relies upon using
>         non-prefixed values with the generic load command, since it
>         doesn't work like that right now.
> 
> [1] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/171172

Acked-by: Stephen Warren <swarren@nvidia.com>

[U-Boot] [PATCH] Fix number base handling of "load" command

[Wolfgang Denk]

Dear Stephen,

In message <5252DDD4.8030300@wwwdotorg.org> you wrote:
> On 10/05/2013 01:07 PM, Wolfgang Denk wrote:
> > A documented, almost all U-Boot commands expect numbers to be entered

Thanks for pointing out.

> s/A/As/ (btw, where?)

In TFM, section "5.9. U-Boot Command Line Interface", note 2 (see [1]):

        ALERT! Almost all U-Boot commands expect numbers to be
        entered in hexadecimal input format. (Exception: for
        historical reasons, the sleep command takes its argument in
        decimal input format.)

[1] http://www.denx.de/wiki/view/DULG/UBootCommandLineInterface

> Acked-by: Stephen Warren <swarren@nvidia.com>

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Anything free is worth what you pay for it.

[U-Boot] [PATCH V2] Fix number base handling of "load" command

[Wolfgang Denk]

As documented, almost all U-Boot commands expect numbers to be
entered in hexadecimal input format.  (Exception: for historical
reasons, the "sleep" command takes its argument in decimal input
format.)

This rule was broken for the "load" command; for details please see
especially commits 045fa1e "fs: add filesystem switch libary,
implement ls and fsload commands" and 3f83c87 "fs: fix number base
behaviour change in fatload/ext*load".  In the result, the load
command would always require an explicit "0x" prefix for regular
(i. e. base 16 formatted) input.

Change this to use the standard notation of base 16 input format.
While strictly speaking this is a change of the user interface, we
hope that it will not cause trouble.  Stephen Warren comments (see
[1]):

        I suppose you can change the behaviour if you want; anyone
        writing "0x..." for their values presumably won't be
        affected, and if people really do assume all values in U-Boot
        are in hex, presumably nobody currently relies upon using
        non-prefixed values with the generic load command, since it
        doesn't work like that right now.

[1] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/171172

Signed-off-by: Wolfgang Denk <wd@denx.de>
cc: Tom Rini <trini@ti.com>, u-boot at lists.denx.de
cc: Stephen Warren <swarren@nvidia.com>
---
Changes in V2: Fix typo in commit message.

The patch has been build-tested (running MAKEALL for Power and ARM
architectures) only.


 common/cmd_ext2.c |  5 ++---
 common/cmd_ext4.c |  5 ++---
 common/cmd_fat.c  |  5 ++---
 common/cmd_fs.c   |  6 ++----
 fs/fs.c           | 16 ++++++++--------
 include/fs.h      |  4 ++--
 6 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/common/cmd_ext2.c b/common/cmd_ext2.c
index 1731919..5a4bcc1 100644
--- a/common/cmd_ext2.c
+++ b/common/cmd_ext2.c
@@ -32,7 +32,7 @@ int do_ext2ls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
  */
 int do_ext2load (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	return do_load(cmdtp, flag, argc, argv, FS_TYPE_EXT, 16);
+	return do_load(cmdtp, flag, argc, argv, FS_TYPE_EXT);
 }
 
 U_BOOT_CMD(
@@ -47,6 +47,5 @@ U_BOOT_CMD(
 	"load binary file from a Ext2 filesystem",
 	"<interface> <dev[:part]> [addr] [filename] [bytes]\n"
 	"    - load binary file 'filename' from 'dev' on 'interface'\n"
-	"      to address 'addr' from ext2 filesystem.\n"
-	"      All numeric parameters are assumed to be hex."
+	"      to address 'addr' from ext2 filesystem."
 );
diff --git a/common/cmd_ext4.c b/common/cmd_ext4.c
index 4a27cd9..8289d25 100644
--- a/common/cmd_ext4.c
+++ b/common/cmd_ext4.c
@@ -45,7 +45,7 @@
 int do_ext4_load(cmd_tbl_t *cmdtp, int flag, int argc,
 						char *const argv[])
 {
-	return do_load(cmdtp, flag, argc, argv, FS_TYPE_EXT, 16);
+	return do_load(cmdtp, flag, argc, argv, FS_TYPE_EXT);
 }
 
 int do_ext4_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
@@ -122,5 +122,4 @@ U_BOOT_CMD(ext4load, 6, 0, do_ext4_load,
 	   "load binary file from a Ext4 filesystem",
 	   "<interface> <dev[:part]> [addr] [filename] [bytes]\n"
 	   "    - load binary file 'filename' from 'dev' on 'interface'\n"
-	   "      to address 'addr' from ext4 filesystem.\n"
-	   "      All numeric parameters are assumed to be hex.");
+	   "      to address 'addr' from ext4 filesystem");
diff --git a/common/cmd_fat.c b/common/cmd_fat.c
index f6d7aff..a12d8fa 100644
--- a/common/cmd_fat.c
+++ b/common/cmd_fat.c
@@ -19,7 +19,7 @@
 
 int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	return do_load(cmdtp, flag, argc, argv, FS_TYPE_FAT, 16);
+	return do_load(cmdtp, flag, argc, argv, FS_TYPE_FAT);
 }
 
 
@@ -35,8 +35,7 @@ U_BOOT_CMD(
 	"      the load stops on end of file.\n"
 	"      If either 'pos' or 'bytes' are not aligned to\n"
 	"      ARCH_DMA_MINALIGN then a misaligned buffer warning will\n"
-	"      be printed and performance will suffer for the load.\n"
-	"      All numeric parameters are assumed to be hex."
+	"      be printed and performance will suffer for the load."
 );
 
 static int do_fat_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
diff --git a/common/cmd_fs.c b/common/cmd_fs.c
index a681d03..91a205a 100644
--- a/common/cmd_fs.c
+++ b/common/cmd_fs.c
@@ -22,7 +22,7 @@
 
 int do_load_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	return do_load(cmdtp, flag, argc, argv, FS_TYPE_ANY, 0);
+	return do_load(cmdtp, flag, argc, argv, FS_TYPE_ANY);
 }
 
 U_BOOT_CMD(
@@ -34,9 +34,7 @@ U_BOOT_CMD(
 	"      'bytes' gives the size to load in bytes.\n"
 	"      If 'bytes' is 0 or omitted, the file is read until the end.\n"
 	"      'pos' gives the file byte position to start reading from.\n"
-	"      If 'pos' is 0 or omitted, the file is read from the start.\n"
-	"      All numeric parameters are assumed to be decimal,\n"
-	"      unless specified otherwise using a leading \"0x\"."
+	"      If 'pos' is 0 or omitted, the file is read from the start."
 );
 
 int do_ls_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
diff --git a/fs/fs.c b/fs/fs.c
index 99e516a..be1855d 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -231,7 +231,7 @@ int fs_write(const char *filename, ulong addr, int offset, int len)
 }
 
 int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
-		int fstype, int cmdline_base)
+		int fstype)
 {
 	unsigned long addr;
 	const char *addr_str;
@@ -250,7 +250,7 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		return 1;
 
 	if (argc >= 4) {
-		addr = simple_strtoul(argv[3], NULL, cmdline_base);
+		addr = simple_strtoul(argv[3], NULL, 16);
 	} else {
 		addr_str = getenv("loadaddr");
 		if (addr_str != NULL)
@@ -268,11 +268,11 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		}
 	}
 	if (argc >= 6)
-		bytes = simple_strtoul(argv[5], NULL, cmdline_base);
+		bytes = simple_strtoul(argv[5], NULL, 16);
 	else
 		bytes = 0;
 	if (argc >= 7)
-		pos = simple_strtoul(argv[6], NULL, cmdline_base);
+		pos = simple_strtoul(argv[6], NULL, 16);
 	else
 		pos = 0;
 
@@ -313,7 +313,7 @@ int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 }
 
 int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
-		int fstype, int cmdline_base)
+		int fstype)
 {
 	unsigned long addr;
 	const char *filename;
@@ -329,10 +329,10 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		return 1;
 
 	filename = argv[3];
-	addr = simple_strtoul(argv[4], NULL, cmdline_base);
-	bytes = simple_strtoul(argv[5], NULL, cmdline_base);
+	addr = simple_strtoul(argv[4], NULL, 16);
+	bytes = simple_strtoul(argv[5], NULL, 16);
 	if (argc >= 7)
-		pos = simple_strtoul(argv[6], NULL, cmdline_base);
+		pos = simple_strtoul(argv[6], NULL, 16);
 	else
 		pos = 0;
 
diff --git a/include/fs.h b/include/fs.h
index c837bae..7d9403e 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -59,10 +59,10 @@ int fs_read(const char *filename, ulong addr, int offset, int len);
  * to a specific filesystem type via the fstype parameter.
  */
 int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
-		int fstype, int cmdline_base);
+		int fstype);
 int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		int fstype);
 int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
-		int fstype, int cmdline_base);
+		int fstype);
 
 #endif /* _FS_H */
-- 
1.8.3.1

[U-Boot] [PATCH] Fix number base handling of "load" command

[Tom Rini]

On Sat, Oct 05, 2013 at 09:07:25PM +0200, Wolfgang Denk wrote:

> A documented, almost all U-Boot commands expect numbers to be entered
> in hexadecimal input format. (Exception: for historical reasons, the
> "sleep" command takes its argument in decimal input format.)
> 
> This rule was broken for the "load" command; for details please see
> especially commits 045fa1e "fs: add filesystem switch libary,
> implement ls and fsload commands" and 3f83c87 "fs: fix number base
> behaviour change in fatload/ext*load".  In the result, the load
> command would always require an explicit "0x" prefix for regular
> (i. e. base 16 formatted) input.
> 
> Change this to use the standard notation of base 16 input format.
> While strictly speaking this is a change of the user interface, we
> hope that it will not cause trouble.  Stephen Warren comments (see
> [1]):
> 
>         I suppose you can change the behaviour if you want; anyone
>         writing "0x..." for their values presumably won't be
>         affected, and if people really do assume all values in U-Boot
>         are in hex, presumably nobody currently relies upon using
>         non-prefixed values with the generic load command, since it
>         doesn't work like that right now.
> 
> [1] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/171172
> 
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> cc: Tom Rini <trini@ti.com>, u-boot at lists.denx.de
> cc: Stephen Warren <swarren@nvidia.com>

Manually correcting the commit message typo, applied to u-boot/master,
thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20131007/753d7825/attachment.pgp>