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

Hi,

On 3 November 2014 18:49, Suriyan Ramasami suriyan.r@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@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