[U-Boot] [PATCH 1/5] fs: fix generic save command implementation

From: Stephen Warren swarren@nvidia.com
Fix a few issues with the generic "save" shell command, and fs_write() function.
1) fstypes[].write wasn't filled in for some file-systems, and isn't checked when used, which could cause crashes/... if executing save on e.g. fat/ext filesystems.
2) fs_write() requires the length argument to be non-zero, since it needs to know exactly how many bytes to write. Adjust the comments and code according to this.
3) fs_write() wasn't prototyped in <fs.h> like other generic functions; other code should be able to call this directly rather than invoking the "save" shell command.
Cc: Simon Glass sjg@chromium.org Signed-off-by: Stephen Warren swarren@nvidia.com --- fs/fs.c | 9 +++------ include/fs.h | 10 ++++++++++ 2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/fs/fs.c b/fs/fs.c index be1855d1291f..9c2ef6b6597c 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -75,6 +75,7 @@ static struct fstype_info fstypes[] = { .close = fat_close, .ls = file_fat_ls, .read = fat_read_file, + .write = fs_write_unsupported, }, #endif #ifdef CONFIG_FS_EXT4 @@ -84,6 +85,7 @@ static struct fstype_info fstypes[] = { .close = ext4fs_close, .ls = ext4fs_ls, .read = ext4_read_file, + .write = fs_write_unsupported, }, #endif #ifdef CONFIG_SANDBOX @@ -212,16 +214,11 @@ int fs_write(const char *filename, ulong addr, int offset, int len) void *buf; int ret;
- /* - * We don't actually know how many bytes are being read, since len==0 - * means read the whole file. - */ buf = map_sysmem(addr, len); ret = info->write(filename, buf, offset, len); unmap_sysmem(buf);
- /* If we requested a specific number of bytes, check we got it */ - if (ret >= 0 && len && ret != len) { + if (ret >= 0 && ret != len) { printf("** Unable to write file %s **\n", filename); ret = -1; } diff --git a/include/fs.h b/include/fs.h index 7d9403ed8758..97b0094e954b 100644 --- a/include/fs.h +++ b/include/fs.h @@ -55,6 +55,16 @@ int fs_ls(const char *dirname); int fs_read(const char *filename, ulong addr, int offset, int len);
/* + * Write file "filename" to the partition previously set by fs_set_blk_dev(), + * from address "addr", starting at byte offset "offset", and writing "len" + * bytes. "offset" may be 0 to write to the start of the file. Note that not + * all filesystem types support offset!=0. + * + * Returns number of bytes read on success. Returns <= 0 on error. + */ +int fs_write(const char *filename, ulong addr, int offset, int len); + +/* * Common implementation for various filesystem commands, optionally limited * to a specific filesystem type via the fstype parameter. */

From: Stephen Warren swarren@nvidia.com
This could be used in scripts such as:
if exists mmc 0:1 /boot/boot.scr; then load mmc 0:1 ${scriptaddr} /boot/boot.scr source ${scriptaddr} fi
rather than:
if load mmc 0:1 ${scriptaddr} /boot/boot.scr; then source ${scriptaddr} fi
This prevents errors being printed by attempts to load non-existent files, which can be important when checking for a large set of files, such as /boot/boot.scr.uimg, /boot/boot.scr, /boot/extlinux.conf, /boot.scr.uimg, /boot.scr, /extlinux.conf.
Signed-off-by: Stephen Warren swarren@nvidia.com --- common/cmd_fs.c | 14 ++++++++++++++ fs/fs.c | 38 ++++++++++++++++++++++++++++++++++++++ include/fs.h | 10 ++++++++++ 3 files changed, 62 insertions(+)
diff --git a/common/cmd_fs.c b/common/cmd_fs.c index 91a205ac1e69..44b00cd125b7 100644 --- a/common/cmd_fs.c +++ b/common/cmd_fs.c @@ -49,3 +49,17 @@ U_BOOT_CMD( " - List files in directory 'directory' of partition 'part' on\n" " device type 'interface' instance 'dev'." ); + +int do_exists_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + return do_exists(cmdtp, flag, argc, argv, FS_TYPE_ANY); +} + +U_BOOT_CMD( + exists, 4, 0, do_exists_wrapper, + "determine whether a file exists", + "<interface> <dev[:part]> filename\n" + " - Determine whether 'filename' exists in partition 'part' on\n" + " device type 'interface' instance 'dev', and set the command.\n" + " exit status so that 'if exists ...; then' works.\n" +); diff --git a/fs/fs.c b/fs/fs.c index 9c2ef6b6597c..f3d9a1c6bc7d 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -41,6 +41,11 @@ static inline int fs_ls_unsupported(const char *dirname) return -1; }
+static inline int fs_exists_unsupported(const char *filename) +{ + return -1; +} + static inline int fs_read_unsupported(const char *filename, void *buf, int offset, int len) { @@ -62,6 +67,7 @@ struct fstype_info { int (*probe)(block_dev_desc_t *fs_dev_desc, disk_partition_t *fs_partition); int (*ls)(const char *dirname); + int (*exists)(const char *filename); int (*read)(const char *filename, void *buf, int offset, int len); int (*write)(const char *filename, void *buf, int offset, int len); void (*close)(void); @@ -74,6 +80,7 @@ static struct fstype_info fstypes[] = { .probe = fat_set_blk_dev, .close = fat_close, .ls = file_fat_ls, + .exists = fs_exists_unsupported, .read = fat_read_file, .write = fs_write_unsupported, }, @@ -84,6 +91,7 @@ static struct fstype_info fstypes[] = { .probe = ext4fs_probe, .close = ext4fs_close, .ls = ext4fs_ls, + .exists = fs_exists_unsupported, .read = ext4_read_file, .write = fs_write_unsupported, }, @@ -94,6 +102,7 @@ static struct fstype_info fstypes[] = { .probe = sandbox_fs_set_blk_dev, .close = sandbox_fs_close, .ls = sandbox_fs_ls, + .exists = fs_exists_unsupported, .read = fs_read_sandbox, .write = fs_write_sandbox, }, @@ -103,6 +112,7 @@ static struct fstype_info fstypes[] = { .probe = fs_probe_unsupported, .close = fs_close_unsupported, .ls = fs_ls_unsupported, + .exists = fs_exists_unsupported, .read = fs_read_unsupported, .write = fs_write_unsupported, }, @@ -184,6 +194,19 @@ int fs_ls(const char *dirname) return ret; }
+int fs_exists(const char *filename) +{ + int ret; + + struct fstype_info *info = fs_get_info(fs_type); + + ret = info->exists(filename); + + fs_close(); + + return ret; +} + int fs_read(const char *filename, ulong addr, int offset, int len) { struct fstype_info *info = fs_get_info(fs_type); @@ -309,6 +332,21 @@ int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], return 0; }
+int do_exists(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], + int fstype) +{ + if (argc != 4) + return CMD_RET_USAGE; + + if (fs_set_blk_dev(argv[1], argv[2], fstype)) + return 1; + + if (fs_exists(argv[3])) + return 1; + + return 0; +} + int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], int fstype) { diff --git a/include/fs.h b/include/fs.h index 97b0094e954b..b8b7706410ad 100644 --- a/include/fs.h +++ b/include/fs.h @@ -44,6 +44,14 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype); int fs_ls(const char *dirname);
/* + * Determine whether a file exists + * + * Returns 0 if the file exists, non-zero if it doesn't exist. + * This encoding was picked to help shell command implementation. + */ +int fs_exists(const char *filename); + +/* * Read file "filename" from the partition previously set by fs_set_blk_dev(), * to address "addr", starting at byte offset "offset", and reading "len" * bytes. "offset" may be 0 to read from the start of the file. "len" may be @@ -72,6 +80,8 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], int fstype); int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], int fstype); +int do_exists(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);

From: Stephen Warren swarren@nvidia.com
This hooks into the generic "file exists" support added in the previous patch, and provides an implementation for the sandbox test environment.
Signed-off-by: Stephen Warren swarren@nvidia.com --- common/cmd_sandbox.c | 7 +++++++ fs/fs.c | 2 +- fs/sandbox/sandboxfs.c | 8 ++++++++ include/sandboxfs.h | 1 + 4 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/common/cmd_sandbox.c b/common/cmd_sandbox.c index 00982b164dd3..c8d36b747d6a 100644 --- a/common/cmd_sandbox.c +++ b/common/cmd_sandbox.c @@ -22,6 +22,12 @@ static int do_sandbox_ls(cmd_tbl_t *cmdtp, int flag, int argc, return do_ls(cmdtp, flag, argc, argv, FS_TYPE_SANDBOX); }
+static int do_sandbox_exists(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[]) +{ + return do_exists(cmdtp, flag, argc, argv, FS_TYPE_SANDBOX); +} + static int do_sandbox_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { @@ -88,6 +94,7 @@ static int do_sandbox_info(cmd_tbl_t *cmdtp, int flag, int argc, static cmd_tbl_t cmd_sandbox_sub[] = { U_BOOT_CMD_MKENT(load, 7, 0, do_sandbox_load, "", ""), U_BOOT_CMD_MKENT(ls, 3, 0, do_sandbox_ls, "", ""), + U_BOOT_CMD_MKENT(exists, 3, 0, do_sandbox_exists, "", ""), U_BOOT_CMD_MKENT(save, 6, 0, do_sandbox_save, "", ""), U_BOOT_CMD_MKENT(bind, 3, 0, do_sandbox_bind, "", ""), U_BOOT_CMD_MKENT(info, 3, 0, do_sandbox_info, "", ""), diff --git a/fs/fs.c b/fs/fs.c index f3d9a1c6bc7d..4f344c6d6d72 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -102,7 +102,7 @@ static struct fstype_info fstypes[] = { .probe = sandbox_fs_set_blk_dev, .close = sandbox_fs_close, .ls = sandbox_fs_ls, - .exists = fs_exists_unsupported, + .exists = sandbox_fs_exists, .read = fs_read_sandbox, .write = fs_write_sandbox, }, diff --git a/fs/sandbox/sandboxfs.c b/fs/sandbox/sandboxfs.c index dd028da8e32b..7940c93dc6a3 100644 --- a/fs/sandbox/sandboxfs.c +++ b/fs/sandbox/sandboxfs.c @@ -72,6 +72,14 @@ int sandbox_fs_ls(const char *dirname) return 0; }
+int sandbox_fs_exists(const char *filename) +{ + ssize_t sz; + + sz = os_get_filesize(filename); + return (sz >= 0) ? 0 : -1; +} + void sandbox_fs_close(void) { } diff --git a/include/sandboxfs.h b/include/sandboxfs.h index 8ea8cb7e2e62..a51ad13044e1 100644 --- a/include/sandboxfs.h +++ b/include/sandboxfs.h @@ -25,6 +25,7 @@ long sandbox_fs_read_at(const char *filename, unsigned long pos,
void sandbox_fs_close(void); int sandbox_fs_ls(const char *dirname); +int sandbox_fs_exists(const char *filename); int fs_read_sandbox(const char *filename, void *buf, int offset, int len); int fs_write_sandbox(const char *filename, void *buf, int offset, int len);

From: Stephen Warren swarren@nvidia.com
This hooks into the generic "file exists" support added in an earlier patch, and provides an implementation for the ext4 filesystem.
Signed-off-by: Stephen Warren swarren@nvidia.com --- fs/ext4/ext4fs.c | 8 ++++++++ fs/fs.c | 2 +- include/ext4fs.h | 1 + 3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 735b2564175b..71ecdd8e9a31 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -174,6 +174,14 @@ int ext4fs_ls(const char *dirname) return 0; }
+int ext4fs_exists(const char *filename) +{ + int file_len; + + file_len = ext4fs_open(filename); + return (file_len >= 0) ? 0 : 1; +} + int ext4fs_read(char *buf, unsigned len) { if (ext4fs_root == NULL || ext4fs_file == NULL) diff --git a/fs/fs.c b/fs/fs.c index 4f344c6d6d72..3f14d0169b43 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -91,7 +91,7 @@ static struct fstype_info fstypes[] = { .probe = ext4fs_probe, .close = ext4fs_close, .ls = ext4fs_ls, - .exists = fs_exists_unsupported, + .exists = ext4fs_exists, .read = ext4_read_file, .write = fs_write_unsupported, }, diff --git a/include/ext4fs.h b/include/ext4fs.h index 242938039662..aacb147de24b 100644 --- a/include/ext4fs.h +++ b/include/ext4fs.h @@ -134,6 +134,7 @@ int ext4fs_read(char *buf, unsigned len); int ext4fs_mount(unsigned part_length); void ext4fs_close(void); int ext4fs_ls(const char *dirname); +int ext4fs_exists(const char *filename); void ext4fs_free_node(struct ext2fs_node *node, struct ext2fs_node *currroot); int ext4fs_devread(lbaint_t sector, int byte_offset, int byte_len, char *buf); void ext4fs_set_blk_dev(block_dev_desc_t *rbdd, disk_partition_t *info);

From: Stephen Warren swarren@nvidia.com
This hooks into the generic "file exists" support added in an earlier patch, and provides an implementation for the ext4 filesystem.
Signed-off-by: Stephen Warren swarren@nvidia.com --- fs/fat/fat.c | 18 ++++++++++++++---- fs/fs.c | 2 +- include/fat.h | 1 + 3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index b41d62e3c386..bc06c0a3c688 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -808,7 +808,7 @@ __u8 do_fat_read_at_block[MAX_CLUSTSIZE]
long do_fat_read_at(const char *filename, unsigned long pos, void *buffer, - unsigned long maxsize, int dols) + unsigned long maxsize, int dols, int dogetsize) { char fnamecopy[2048]; boot_sector bs; @@ -1152,7 +1152,10 @@ rootdir_done: subname = nextname; }
- ret = get_contents(mydata, dentptr, pos, buffer, maxsize); + if (dogetsize) + ret = FAT2CPU32(dentptr->size); + else + ret = get_contents(mydata, dentptr, pos, buffer, maxsize); debug("Size: %d, got: %ld\n", FAT2CPU32(dentptr->size), ret);
exit: @@ -1163,7 +1166,7 @@ exit: long do_fat_read(const char *filename, void *buffer, unsigned long maxsize, int dols) { - return do_fat_read_at(filename, 0, buffer, maxsize, dols); + return do_fat_read_at(filename, 0, buffer, maxsize, dols, 0); }
int file_fat_detectfs(void) @@ -1233,11 +1236,18 @@ int file_fat_ls(const char *dir) return do_fat_read(dir, NULL, 0, LS_YES); }
+int fat_exists(const char *filename) +{ + int sz; + sz = do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1); + return (sz >= 0) ? 0 : 1; +} + long file_fat_read_at(const char *filename, unsigned long pos, void *buffer, unsigned long maxsize) { printf("reading %s\n", filename); - return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO); + return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO, 0); }
long file_fat_read(const char *filename, void *buffer, unsigned long maxsize) diff --git a/fs/fs.c b/fs/fs.c index 3f14d0169b43..d2bc8d0f1459 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -80,7 +80,7 @@ static struct fstype_info fstypes[] = { .probe = fat_set_blk_dev, .close = fat_close, .ls = file_fat_ls, - .exists = fs_exists_unsupported, + .exists = fat_exists, .read = fat_read_file, .write = fs_write_unsupported, }, diff --git a/include/fat.h b/include/fat.h index 2c951e7d79c6..c8eb7ccd2904 100644 --- a/include/fat.h +++ b/include/fat.h @@ -188,6 +188,7 @@ file_read_func file_fat_read; int file_cd(const char *path); int file_fat_detectfs(void); int file_fat_ls(const char *dir); +int fat_exists(const char *filename); long file_fat_read_at(const char *filename, unsigned long pos, void *buffer, unsigned long maxsize); long file_fat_read(const char *filename, void *buffer, unsigned long maxsize);

Hi Stephen,
On 16.01.2014 22:53, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
This hooks into the generic "file exists" support added in an earlier patch, and provides an implementation for the ext4 filesystem.
Signed-off-by: Stephen Warren swarren@nvidia.com
fs/fat/fat.c | 18 ++++++++++++++---- fs/fs.c | 2 +- include/fat.h | 1 + 3 files changed, 16 insertions(+), 5 deletions(-)
Nitpicking, but the subject and the description mention "ext4fs" (copy and paste mistake). Please change it to "fat" to match the patch.
Thanks, Stefan

On 01/16/2014 10:54 PM, Stefan Roese wrote:
Hi Stephen,
On 16.01.2014 22:53, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
This hooks into the generic "file exists" support added in an earlier patch, and provides an implementation for the ext4 filesystem.
Signed-off-by: Stephen Warren swarren@nvidia.com
fs/fat/fat.c | 18 ++++++++++++++---- fs/fs.c | 2 +- include/fat.h | 1 + 3 files changed, 16 insertions(+), 5 deletions(-)
Nitpicking, but the subject and the description mention "ext4fs" (copy and paste mistake). Please change it to "fat" to match the patch.
Thanks for pointing that out. I've fixed it locally. Once the series is reviewed/... I can repost with the fix.
participants (2)
-
Stefan Roese
-
Stephen Warren