Hi Stephen,
On Monday, October 22, 2012 6:43:51 PM, Stephen Warren wrote:
> From: Stephen Warren <swar...@nvidia.com>
>
> Implement "ls" and "fsload" commands that act like
> {fat,ext2}{ls,load},
> and transparently handle either file-system. This scheme could easily
> be
> extended to other filesystem types; I only didn't do it for zfs
> because
> I don't have any filesystems of that type to test with.
>
> Replace the implementation of {fat,ext[24]}{ls,load} with this new
> code
> too.
>
> Signed-off-by: Stephen Warren <swar...@nvidia.com>
Sorry to come after this has been applied. I've been ultra busy lately.
[--snip--]
> diff --git a/fs/fs.c b/fs/fs.c
> new file mode 100644
> index 0000000..23ffa25
> --- /dev/null
> +++ b/fs/fs.c
> @@ -0,0 +1,308 @@
[--snip--]
> +int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[],
> + int fstype)
> +{
> + unsigned long addr;
> + const char *addr_str;
> + const char *filename;
> + unsigned long bytes;
> + unsigned long pos;
> + int len_read;
> + char buf[12];
> +
> + if (argc < 5)
With the arguments now made optional, this should rather be:
+ if (argc < 2)
> + return CMD_RET_USAGE;
> +
> + if (fs_set_blk_dev(argv[1], argv[2], fstype))
With <dev[:part]> being optional, this should rather be:
+ if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL, fstype))
> + return 1;
> +
> + if (argc >= 4) {
> + addr = simple_strtoul(argv[3], NULL, 0);
0 is just natural here. However, this raises the issue of the users of the
legacy fat and ext commands, which used 16 here. So should we use 0 because it
is cleaner, or 16 in order not to break compatibility for existing users?
> + } else {
> + addr_str = getenv("loadaddr");
> + if (addr_str != NULL)
> + addr = simple_strtoul(addr_str, NULL, 16);
Ditto.
> + else
> + addr = CONFIG_SYS_LOAD_ADDR;
> + }
> + if (argc >= 5) {
> + filename = argv[4];
> + } else {
> + filename = getenv("bootfile");
> + if (!filename) {
> + puts("** No boot file defined **\n");
> + return 1;
> + }
> + }
> + if (argc >= 6)
> + bytes = simple_strtoul(argv[5], NULL, 0);
Ditto.
> + else
> + bytes = 0;
> + if (argc >= 7)
> + pos = simple_strtoul(argv[6], NULL, 0);
Ditto.
> + else
> + pos = 0;
> +
> + len_read = fs_read(filename, addr, pos, bytes);
> + if (len_read <= 0)
> + return 1;
> +
> + printf("%d bytes read\n", len_read);
> +
> + sprintf(buf, "0x%x", len_read);
> + setenv("filesize", buf);
> +
> + return 0;
> +}
> +
> +int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> + int fstype)
> +{
> + if (argc < 2)
> + 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] : "/"))
IMHO, it would be better to just ignore the possible extra arguments, like in:
+ if (fs_ls(argc >= 4 ? argv[3] : "/"))
> + return 1;
> +
> + return 0;
> +}
[--snip--]
Best regards,
Benoît
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
