[U-Boot] [PATCH 1/3] fs: fix do_fsload() handling of optional arguments

[U-Boot] [PATCH 1/3] fs: fix do_fsload() handling of optional arguments

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Most arguments to the shell command do_fsload() implements are optional.
Fix the minimum argc check to respect that. Cater for the situation
where argv[2] is not provided.

Enhance both do_fsload() and do_ls() to check the maximum number of
arguments too. While this check would typically be implemented via
U_BOOT_CMD()'s max_args parameter, if these functions are called
directly, then that check won't exist.

Finally, alter do_ls() to check (argc >= 4) rather than (argc == 4) so
that if the function is enhanced to allow extra arguments in the future,
this test won't need to be changed at that time.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 fs/fs.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index e148a07..f570312 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -258,10 +258,12 @@ int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 	int len_read;
 	char buf[12];
 
-	if (argc < 5)
+	if (argc < 2)
+		return CMD_RET_USAGE;
+	if (argc > 7)
 		return CMD_RET_USAGE;
 
-	if (fs_set_blk_dev(argv[1], argv[2], fstype))
+	if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL, fstype))
 		return 1;
 
 	if (argc >= 4) {
@@ -308,11 +310,13 @@ int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 {
 	if (argc < 2)
 		return CMD_RET_USAGE;
+	if (argc > 4)
+		return CMD_RET_USAGE;
 
 	if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL, fstype))
 		return 1;
 
-	if (fs_ls(argc == 4 ? argv[3] : "/"))
+	if (fs_ls(argc >= 4 ? argv[3] : "/"))
 		return 1;
 
 	return 0;
-- 
1.7.0.4

[U-Boot] [PATCH 2/3] cmd_ext4: remove TABs from command help text

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

TABs in the help text won't line up in the same place on the console as
in a source editor. Replace them with spaces to make ensuring correct
alignment easier.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 common/cmd_ext4.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/common/cmd_ext4.c b/common/cmd_ext4.c
index 237bc19..3bd1da3 100644
--- a/common/cmd_ext4.c
+++ b/common/cmd_ext4.c
@@ -123,17 +123,17 @@ fail:
 U_BOOT_CMD(ext4write, 6, 1, do_ext4_write,
 	"create a file in the root directory",
 	"<interface> <dev[:part]> [Absolute filename path] [Address] [sizebytes]\n"
-	"	  - create a file in / directory");
+	"    - create a file in / directory");
 
 #endif
 
 U_BOOT_CMD(ext4ls, 4, 1, do_ext4_ls,
 	   "list files in a directory (default /)",
 	   "<interface> <dev[:part]> [directory]\n"
-	   "	  - list files from 'dev' on 'interface' in a 'directory'");
+	   "    - list files from 'dev' on 'interface' in a 'directory'");
 
 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");
+	   "    - load binary file 'filename' from 'dev' on 'interface'\n"
+	   "      to address 'addr' from ext4 filesystem");
-- 
1.7.0.4

[U-Boot] [PATCH 3/3] fs: fix number base behaviour change in fatload/ext*load

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Commit 045fa1e "fs: add filesystem switch libary, implement ls and
fsload commands" unified the implementation of fatload and ext*load
with the new command fsload. However, this altered the interpretation
of command-line numbers from always being base-16, to requiring a "0x"
prefix for base-16 numbers. Enhance do_fsload() to allow commands to
specify which base to use.

Use base 0, thus requiring a "0x" prefix for the new fsload command.
This feels much cleaner than assuming base 16.

Use base 16 for the pre-existing fatload and ext*load to prevent a
change in behaviour.

Use base 16 exclusively for the loadaddr environment variable, since
that variable is interpreted in multiple places, so we don't want the
behaviour to change.

Update command help text to make it clear where numbers are assumed to
be hex, and where an explicit "0x" prefix is required.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 common/cmd_ext2.c |    5 +++--
 common/cmd_ext4.c |    5 +++--
 common/cmd_fat.c  |    5 +++--
 common/cmd_fs.c   |    6 ++++--
 fs/fs.c           |    8 ++++----
 include/fs.h      |    2 +-
 6 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/common/cmd_ext2.c b/common/cmd_ext2.c
index 06d0234..7f22598 100644
--- a/common/cmd_ext2.c
+++ b/common/cmd_ext2.c
@@ -49,7 +49,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_fsload(cmdtp, flag, argc, argv, FS_TYPE_EXT);
+	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_EXT, 16);
 }
 
 U_BOOT_CMD(
@@ -64,5 +64,6 @@ 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"
+	"      to address 'addr' from ext2 filesystem.\n"
+	"      All numeric parameters are assumed to be hex."
 );
diff --git a/common/cmd_ext4.c b/common/cmd_ext4.c
index 3bd1da3..7864276 100644
--- a/common/cmd_ext4.c
+++ b/common/cmd_ext4.c
@@ -59,7 +59,7 @@
 int do_ext4_load(cmd_tbl_t *cmdtp, int flag, int argc,
 						char *const argv[])
 {
-	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_EXT);
+	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_EXT, 16);
 }
 
 int do_ext4_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
@@ -136,4 +136,5 @@ 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");
+	   "      to address 'addr' from ext4 filesystem.\n"
+	   "      All numeric parameters are assumed to be hex.");
diff --git a/common/cmd_fat.c b/common/cmd_fat.c
index c865d6d..8280483 100644
--- a/common/cmd_fat.c
+++ b/common/cmd_fat.c
@@ -35,7 +35,7 @@
 
 int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_FAT);
+	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_FAT, 16);
 }
 
 
@@ -48,7 +48,8 @@ U_BOOT_CMD(
 	"      'pos' gives the file position to start loading from.\n"
 	"      If 'pos' is omitted, 0 is used. 'pos' requires 'bytes'.\n"
 	"      'bytes' gives the size to load. If 'bytes' is 0 or omitted,\n"
-	"      the load stops on end of file."
+	"      the load stops on end of file.\n"
+	"      All numeric parameters are assumed to be hex."
 );
 
 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 296124b..46fcef7 100644
--- a/common/cmd_fs.c
+++ b/common/cmd_fs.c
@@ -22,7 +22,7 @@
 
 int do_fsload_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_ANY);
+	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_ANY, 0);
 }
 
 U_BOOT_CMD(
@@ -34,7 +34,9 @@ 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."
+	"      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\"."
 );
 
 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 f570312..1553af5 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -248,7 +248,7 @@ int fs_read(const char *filename, ulong addr, int offset, int len)
 }
 
 int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
-		int fstype)
+		int fstype, int cmdline_base)
 {
 	unsigned long addr;
 	const char *addr_str;
@@ -267,7 +267,7 @@ int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		return 1;
 
 	if (argc >= 4) {
-		addr = simple_strtoul(argv[3], NULL, 0);
+		addr = simple_strtoul(argv[3], NULL, cmdline_base);
 	} else {
 		addr_str = getenv("loadaddr");
 		if (addr_str != NULL)
@@ -285,11 +285,11 @@ int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		}
 	}
 	if (argc >= 6)
-		bytes = simple_strtoul(argv[5], NULL, 0);
+		bytes = simple_strtoul(argv[5], NULL, cmdline_base);
 	else
 		bytes = 0;
 	if (argc >= 7)
-		pos = simple_strtoul(argv[6], NULL, 0);
+		pos = simple_strtoul(argv[6], NULL, cmdline_base);
 	else
 		pos = 0;
 
diff --git a/include/fs.h b/include/fs.h
index f396d84..c3ac7cc 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -58,7 +58,7 @@ int fs_read(const char *filename, ulong addr, int offset, int len);
  * to a specific filesystem type via the fstype parameter.
  */
 int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
-		int fstype);
+		int fstype, int cmdline_base);
 int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		int fstype);
 
-- 
1.7.0.4

[U-Boot] [PATCH 1/3] fs: fix do_fsload() handling of optional arguments

[Benoît Thébaudeau]

On Tuesday, October 30, 2012 11:04:17 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Most arguments to the shell command do_fsload() implements are
> optional.
> Fix the minimum argc check to respect that. Cater for the situation
> where argv[2] is not provided.
> 
> Enhance both do_fsload() and do_ls() to check the maximum number of
> arguments too. While this check would typically be implemented via
> U_BOOT_CMD()'s max_args parameter, if these functions are called
> directly, then that check won't exist.
> 
> Finally, alter do_ls() to check (argc >= 4) rather than (argc == 4)
> so
> that if the function is enhanced to allow extra arguments in the
> future,
> this test won't need to be changed at that time.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
[--snip--]

For the series:
Reviewed-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>

Best regards,
Beno?t

[U-Boot] [PATCH 1/3] fs: fix do_fsload() handling of optional arguments

[Tom Rini]

On Tue, Oct 30, 2012 at 11:28:55PM +0100, Beno??t Th??baudeau wrote:
> On Tuesday, October 30, 2012 11:04:17 PM, Stephen Warren wrote:
> > From: Stephen Warren <swarren@nvidia.com>
> > 
> > Most arguments to the shell command do_fsload() implements are
> > optional.
> > Fix the minimum argc check to respect that. Cater for the situation
> > where argv[2] is not provided.
> > 
> > Enhance both do_fsload() and do_ls() to check the maximum number of
> > arguments too. While this check would typically be implemented via
> > U_BOOT_CMD()'s max_args parameter, if these functions are called
> > directly, then that check won't exist.
> > 
> > Finally, alter do_ls() to check (argc >= 4) rather than (argc == 4)
> > so
> > that if the function is enhanced to allow extra arguments in the
> > future,
> > this test won't need to be changed at that time.
> > 
> > Signed-off-by: Stephen Warren <swarren@nvidia.com>
> [--snip--]
> 
> For the series:
> Reviewed-by: Beno??t Th??baudeau <benoit.thebaudeau@advansee.com>

And for the series, 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/20121104/696f70e9/attachment.pgp>