[U-Boot] [PATCH v6 2/6] fs: interface changes to accomodate files greater than 2GB

Simon Glass sjg at chromium.org
Wed Nov 5 01:49:57 CET 2014


Hi,

On 3 November 2014 18:49, Suriyan Ramasami <suriyan.r at gmail.com> wrote:
> Change the interface for the generic FS functions to take in an extra
> parameter of type "loff_t *" to return the size. The return values of
> these funtions now serve as an indicator of error conditions alone.
>
> Signed-off-by: Suriyan Ramasami <suriyan.r at gmail.com>

This looks good apart from Pavel's comments and my two minor nits below.

> ---
>
> Changes in v6:
> * Simon - Split this into a separate patch
>
> Changes in v5:
> * Simon - update fs.h with comments for fs_read/fs_write/fs_size
>
>  common/cmd_fs.c | 17 +++++++++++++
>  fs/fs.c         | 77 ++++++++++++++++++++++++++++++++++-----------------------
>  include/fs.h    | 41 ++++++++++++++++++------------
>  3 files changed, 88 insertions(+), 47 deletions(-)
>
> diff --git a/common/cmd_fs.c b/common/cmd_fs.c
> index 6754340..f70cb8a 100644
> --- a/common/cmd_fs.c
> +++ b/common/cmd_fs.c
> @@ -51,6 +51,23 @@ U_BOOT_CMD(
>         "      If 'pos' is 0 or omitted, the file is read from the start."
>  )
>
> +static int do_save_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
> +                               char * const argv[])
> +{
> +       return do_save(cmdtp, flag, argc, argv, FS_TYPE_ANY);
> +}
> +
> +U_BOOT_CMD(
> +       save,   7,      0,      do_save_wrapper,
> +       "save file to a filesystem",
> +       "<interface> [<dev[:part]> [<addr> [<filename> [bytes [pos]]]]]\n"
> +       "    - Save binary file 'filename' to partition 'part' on device\n"
> +       "      type 'interface' instance 'dev' from addr 'addr' in memory.\n"
> +       "      'bytes' gives the size to save in bytes and is mandatory.\n"
> +       "      'pos' gives the file byte position to start writing to.\n"
> +       "      If 'pos' is 0 or omitted, the file is written from the start."
> +)
> +
>  static 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 dd680f3..0f5a1f4 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -46,19 +46,21 @@ static inline int fs_exists_unsupported(const char *filename)
>         return 0;
>  }
>
> -static inline int fs_size_unsupported(const char *filename)
> +static inline int fs_size_unsupported(const char *filename, loff_t *size)
>  {
>         return -1;
>  }
>
>  static inline int fs_read_unsupported(const char *filename, void *buf,
> -                                     int offset, int len)
> +                                     loff_t offset, loff_t len,
> +                                     loff_t *actread)
>  {
>         return -1;
>  }
>
>  static inline int fs_write_unsupported(const char *filename, void *buf,
> -                                     int offset, int len)
> +                                     loff_t offset, loff_t len,
> +                                     loff_t *actwrite)
>  {
>         return -1;
>  }
> @@ -82,9 +84,11 @@ struct fstype_info {
>                      disk_partition_t *fs_partition);
>         int (*ls)(const char *dirname);
>         int (*exists)(const char *filename);
> -       int (*size)(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);
> +       int (*size)(const char *filename, loff_t *size);
> +       int (*read)(const char *filename, void *buf, loff_t offset,
> +                   loff_t len, loff_t *actread);
> +       int (*write)(const char *filename, void *buf, loff_t offset,
> +                    loff_t len, loff_t *actwrite);
>         void (*close)(void);
>  };
>
> @@ -99,7 +103,11 @@ static struct fstype_info fstypes[] = {
>                 .exists = fat_exists,
>                 .size = fat_size,
>                 .read = fat_read_file,
> +#ifdef CONFIG_FAT_WRITE
> +               .write = file_fat_write,
> +#else
>                 .write = fs_write_unsupported,
> +#endif
>         },
>  #endif
>  #ifdef CONFIG_FS_EXT4
> @@ -112,7 +120,11 @@ static struct fstype_info fstypes[] = {
>                 .exists = ext4fs_exists,
>                 .size = ext4fs_size,
>                 .read = ext4_read_file,
> +#ifdef CONFIG_CMD_EXT4_WRITE
> +               .write = ext4_write_file,
> +#else
>                 .write = fs_write_unsupported,
> +#endif
>         },
>  #endif
>  #ifdef CONFIG_SANDBOX
> @@ -233,20 +245,21 @@ int fs_exists(const char *filename)
>         return ret;
>  }
>
> -int fs_size(const char *filename)
> +int fs_size(const char *filename, loff_t *size)
>  {
>         int ret;
>
>         struct fstype_info *info = fs_get_info(fs_type);
>
> -       ret = info->size(filename);
> +       ret = info->size(filename, size);
>
>         fs_close();
>
>         return ret;
>  }
>
> -int fs_read(const char *filename, ulong addr, int offset, int len)
> +int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
> +           loff_t *actread)
>  {
>         struct fstype_info *info = fs_get_info(fs_type);
>         void *buf;
> @@ -257,11 +270,11 @@ int fs_read(const char *filename, ulong addr, int offset, int len)
>          * means read the whole file.
>          */
>         buf = map_sysmem(addr, len);
> -       ret = info->read(filename, buf, offset, len);
> +       ret = info->read(filename, buf, offset, len, actread);
>         unmap_sysmem(buf);
>
>         /* If we requested a specific number of bytes, check we got it */
> -       if (ret >= 0 && len && ret != len) {
> +       if (ret == 0 && len && *actread != len) {
>                 printf("** Unable to read file %s **\n", filename);
>                 ret = -1;
>         }
> @@ -270,17 +283,18 @@ int fs_read(const char *filename, ulong addr, int offset, int len)
>         return ret;
>  }
>
> -int fs_write(const char *filename, ulong addr, int offset, int len)
> +int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
> +            loff_t *actwrite)
>  {
>         struct fstype_info *info = fs_get_info(fs_type);
>         void *buf;
>         int ret;
>
>         buf = map_sysmem(addr, len);
> -       ret = info->write(filename, buf, offset, len);
> +       ret = info->write(filename, buf, offset, len, actwrite);
>         unmap_sysmem(buf);
>
> -       if (ret >= 0 && ret != len) {
> +       if (ret < 0 && len != *actwrite) {
>                 printf("** Unable to write file %s **\n", filename);
>                 ret = -1;
>         }
> @@ -292,7 +306,7 @@ int fs_write(const char *filename, ulong addr, int offset, int len)
>  int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>                 int fstype)
>  {
> -       int size;
> +       loff_t size;
>
>         if (argc != 4)
>                 return CMD_RET_USAGE;
> @@ -300,8 +314,7 @@ int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>         if (fs_set_blk_dev(argv[1], argv[2], fstype))
>                 return 1;
>
> -       size = fs_size(argv[3]);
> -       if (size < 0)
> +       if (fs_size(argv[3], &size) < 0)
>                 return CMD_RET_FAILURE;
>
>         setenv_hex("filesize", size);
> @@ -315,9 +328,10 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>         unsigned long addr;
>         const char *addr_str;
>         const char *filename;
> -       unsigned long bytes;
> -       unsigned long pos;
> -       int len_read;
> +       loff_t bytes;
> +       loff_t pos;
> +       loff_t len_read;
> +       int ret;
>         unsigned long time;
>         char *ep;
>
> @@ -359,12 +373,12 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>                 pos = 0;
>
>         time = get_timer(0);
> -       len_read = fs_read(filename, addr, pos, bytes);
> +       ret = fs_read(filename, addr, pos, bytes, &len_read);
>         time = get_timer(time);
> -       if (len_read <= 0)
> +       if (ret < 0)
>                 return 1;
>
> -       printf("%d bytes read in %lu ms", len_read, time);
> +       printf("%llu bytes read in %lu ms", len_read, time);
>         if (time > 0) {
>                 puts(" (");
>                 print_size(len_read / time * 1000, "/s");
> @@ -408,9 +422,10 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>  {
>         unsigned long addr;
>         const char *filename;
> -       unsigned long bytes;
> -       unsigned long pos;
> -       int len;
> +       loff_t bytes;
> +       loff_t pos;
> +       loff_t len;
> +       int ret;
>         unsigned long time;
>
>         if (argc < 6 || argc > 7)
> @@ -419,8 +434,8 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>         if (fs_set_blk_dev(argv[1], argv[2], fstype))
>                 return 1;
>
> -       filename = argv[3];
> -       addr = simple_strtoul(argv[4], NULL, 16);
> +       addr = simple_strtoul(argv[3], NULL, 16);
> +       filename = argv[4];
>         bytes = simple_strtoul(argv[5], NULL, 16);
>         if (argc >= 7)
>                 pos = simple_strtoul(argv[6], NULL, 16);
> @@ -428,12 +443,12 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>                 pos = 0;
>
>         time = get_timer(0);
> -       len = fs_write(filename, addr, pos, bytes);
> +       ret = fs_write(filename, addr, pos, bytes, &len);
>         time = get_timer(time);
> -       if (len <= 0)
> +       if (ret < 0)
>                 return 1;
>
> -       printf("%d bytes written in %lu ms", len, time);
> +       printf("%llu bytes written in %lu ms", len, time);
>         if (time > 0) {
>                 puts(" (");
>                 print_size(len / time * 1000, "/s");
> diff --git a/include/fs.h b/include/fs.h
> index 06a45f2..6bd17c8 100644
> --- a/include/fs.h
> +++ b/include/fs.h
> @@ -51,32 +51,41 @@ int fs_ls(const char *dirname);
>  int fs_exists(const char *filename);
>
>  /*
> - * Determine a file's size
> + * fs_size - Determine a file's size
>   *
> - * Returns the file's size in bytes, or a negative value if it doesn't exist.
> + * @filename: Name of the file
> + * @size: Size of file
> + * @return 0 if ok with valid *size, negative on error
>   */
> -int fs_size(const char *filename);
> +int fs_size(const char *filename, loff_t *size);
>
>  /*
> - * 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
> - * 0 to read the entire file. Note that not all filesystem types support
> - * either/both offset!=0 or len!=0.
> + * fs_read - Read file from the partition previously set by fs_set_blk_dev()
> + * Note that not all filesystem types support either/both offset!=0 or len!=0.
>   *
> - * Returns number of bytes read on success. Returns <= 0 on error.
> + * @filename: Name of file to read from
> + * @addr: The address to read into
> + * @offset: The offset in file to read from
> + * @len: The number of bytes to read. Maybe 0 to read entire file
> + * @actread: The actual number of bytes read

Returns the actual number...

> + * @return 0 if ok with valid *actread, -1 on error conditions
>   */
> -int fs_read(const char *filename, ulong addr, int offset, int len);
> +int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
> +           loff_t *actread);
>
>  /*
> - * 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.
> + * fs_write - Write file to the partition previously set by fs_set_blk_dev()
> + * Note that not all filesystem types support offset!=0.
>   *
> - * Returns number of bytes read on success. Returns <= 0 on error.
> + * @filename: Name of file to read from
> + * @addr: The address to read into
> + * @offset: The offset in file to read from. Maybe 0 to write to start of file
> + * @len: The number of bytes to write
> + * @actwrite: The actual number of bytes written

Returns the actual number...

> + * @return 0 if ok with valid *actwrite, -1 on error conditions
>   */
> -int fs_write(const char *filename, ulong addr, int offset, int len);
> +int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
> +            loff_t *actwrite);
>
>  /*
>   * Common implementation for various filesystem commands, optionally limited
> --
> 1.9.1
>

Regards,
Simon


More information about the U-Boot mailing list