[U-Boot] [PATCH v1 1/1] fs: fat/ext4/sandbox: Deal with files > 2GB in ls and size commands
Simon Glass
sjg at chromium.org
Wed Oct 8 22:44:35 CEST 2014
Hi Suriyan,
On 8 October 2014 14:23, Suriyan Ramasami <suriyan.r at gmail.com> wrote:
> The commands fatls/ext4ls give -ve values when dealing with files > 2GB.
> The commands fatsize/ext4size do not update the variable filesize for
> these files.
>
> To deal with this, the functions *_size have been modified to take a second
> parameter of type "* off_t" which is then populated. The return value of the
> *_size function is then only used to determine error conditions.
That seems like a sensible idea to me.
>
> Signed-off-by: Suriyan Ramasami <suriyan.r at gmail.com>
> ---
> arch/sandbox/cpu/os.c | 14 +++++------
> arch/sandbox/cpu/state.c | 6 ++---
> common/board_f.c | 6 ++---
> fs/ext4/ext4_common.c | 13 +++++------
> fs/ext4/ext4fs.c | 21 ++++++++++-------
> fs/fat/fat.c | 61 ++++++++++++++++++++++++++++++++----------------
> fs/fs.c | 15 ++++++------
> fs/sandbox/sandboxfs.c | 23 ++++++++++++------
> include/ext4fs.h | 4 ++--
> include/fat.h | 2 +-
> include/fs.h | 2 +-
> include/os.h | 2 +-
> include/sandboxfs.h | 2 +-
> 13 files changed, 103 insertions(+), 68 deletions(-)
Thanks for doing this. Do you think you could also add a test for this
(perhaps in test/fs). It could create a temporary ext2 filesystem, put
a large file in it and then check a few operations.
>
> diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
> index 1c4aa3f..9b052ee 100644
> --- a/arch/sandbox/cpu/os.c
> +++ b/arch/sandbox/cpu/os.c
> @@ -385,15 +385,15 @@ const char *os_dirent_get_typename(enum os_dirent_t type)
> return os_dirent_typename[OS_FILET_UNKNOWN];
> }
>
> -ssize_t os_get_filesize(const char *fname)
> +int os_get_filesize(const char *fname, off_t *size)
> {
> struct stat buf;
> int ret;
>
> ret = stat(fname, &buf);
> - if (ret)
> - return ret;
> - return buf.st_size;
> + if (ret == 0)
> + *size = buf.st_size;
> + return ret;
> }
>
> void os_putc(int ch)
> @@ -427,10 +427,10 @@ int os_read_ram_buf(const char *fname)
> {
> struct sandbox_state *state = state_get_current();
> int fd, ret;
> - int size;
> + off_t size;
>
> - size = os_get_filesize(fname);
> - if (size < 0)
> + ret = os_get_filesize(fname, &size);
> + if (ret < 0)
> return -ENOENT;
Should you return ret instead here (and in other cases below)?
> if (size != state->ram_size)
> return -ENOSPC;
> diff --git a/arch/sandbox/cpu/state.c b/arch/sandbox/cpu/state.c
> index 59adad6..e0d119a 100644
> --- a/arch/sandbox/cpu/state.c
> +++ b/arch/sandbox/cpu/state.c
> @@ -49,12 +49,12 @@ static int state_ensure_space(int extra_size)
>
> static int state_read_file(struct sandbox_state *state, const char *fname)
> {
> - int size;
> + off_t size;
> int ret;
> int fd;
>
> - size = os_get_filesize(fname);
> - if (size < 0) {
> + ret = os_get_filesize(fname, &size);
> + if (ret < 0) {
> printf("Cannot find sandbox state file '%s'\n", fname);
> return -ENOENT;
> }
> diff --git a/common/board_f.c b/common/board_f.c
> index e6aa298..b8ec7ac 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -291,7 +291,7 @@ static int read_fdt_from_file(void)
> struct sandbox_state *state = state_get_current();
> const char *fname = state->fdt_fname;
> void *blob;
> - ssize_t size;
> + off_t size;
> int err;
> int fd;
>
> @@ -304,8 +304,8 @@ static int read_fdt_from_file(void)
> return -EINVAL;
> }
>
> - size = os_get_filesize(fname);
> - if (size < 0) {
> + err = os_get_filesize(fname, &size);
> + if (err < 0) {
> printf("Failed to file FDT file '%s'\n", fname);
> return -ENOENT;
> }
> diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
> index 33d69c9..fd9b611 100644
> --- a/fs/ext4/ext4_common.c
> +++ b/fs/ext4/ext4_common.c
> @@ -2003,9 +2003,9 @@ int ext4fs_iterate_dir(struct ext2fs_node *dir, char *name,
> printf("< ? > ");
> break;
> }
> - printf("%10d %s\n",
> - __le32_to_cpu(fdiro->inode.size),
> - filename);
> + printf("%10u %s\n",
> + __le32_to_cpu(fdiro->inode.size),
> + filename);
> }
> free(fdiro);
> }
> @@ -2169,11 +2169,10 @@ int ext4fs_find_file(const char *path, struct ext2fs_node *rootnode,
> return 1;
> }
>
> -int ext4fs_open(const char *filename)
> +int ext4fs_open(const char *filename, off_t *len)
> {
> struct ext2fs_node *fdiro = NULL;
> int status;
> - int len;
>
> if (ext4fs_root == NULL)
> return -1;
> @@ -2190,10 +2189,10 @@ int ext4fs_open(const char *filename)
> if (status == 0)
> goto fail;
> }
> - len = __le32_to_cpu(fdiro->inode.size);
> + *len = __le32_to_cpu(fdiro->inode.size);
> ext4fs_file = fdiro;
>
> - return len;
> + return 0;
> fail:
> ext4fs_free_node(fdiro, &ext4fs_root->diropen);
>
> diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c
> index cbdc220..3e4eaaa 100644
> --- a/fs/ext4/ext4fs.c
> +++ b/fs/ext4/ext4fs.c
> @@ -176,15 +176,19 @@ int ext4fs_ls(const char *dirname)
>
> int ext4fs_exists(const char *filename)
> {
> - int file_len;
> + off_t file_len;
> + int ret;
>
> - file_len = ext4fs_open(filename);
> - return file_len >= 0;
> + ret = ext4fs_open(filename, &file_len);
> + return ret == 0;
> }
>
> -int ext4fs_size(const char *filename)
> +int ext4fs_size(const char *filename, off_t *file_size)
> {
> - return ext4fs_open(filename);
> + int ret;
> +
> + ret = ext4fs_open(filename, file_size);
> + return ret == 0;
> }
>
> int ext4fs_read(char *buf, unsigned len)
> @@ -210,7 +214,8 @@ int ext4fs_probe(block_dev_desc_t *fs_dev_desc,
>
> int ext4_read_file(const char *filename, void *buf, int offset, int len)
> {
> - int file_len;
> + off_t file_len;
> + int ret;
> int len_read;
>
> if (offset != 0) {
> @@ -218,8 +223,8 @@ int ext4_read_file(const char *filename, void *buf, int offset, int len)
> return -1;
> }
>
> - file_len = ext4fs_open(filename);
> - if (file_len < 0) {
> + ret = ext4fs_open(filename, &file_len);
> + if (ret != 0) {
> printf("** File not found %s **\n", filename);
> return -1;
> }
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 561921f..650ee7e 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -806,9 +806,9 @@ exit:
> __u8 do_fat_read_at_block[MAX_CLUSTSIZE]
> __aligned(ARCH_DMA_MINALIGN);
>
> -long
> +int
> do_fat_read_at(const char *filename, unsigned long pos, void *buffer,
> - unsigned long maxsize, int dols, int dogetsize)
> + unsigned long maxsize, int dols, int dogetsize, off_t *size)
> {
> char fnamecopy[2048];
> boot_sector bs;
> @@ -974,10 +974,10 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer,
> }
> if (doit) {
> if (dirc == ' ') {
> - printf(" %8ld %s%c\n",
> - (long)FAT2CPU32(dentptr->size),
> - l_name,
> - dirc);
> + printf(" %8u %s%c\n",
> + FAT2CPU32(dentptr->size),
> + l_name,
> + dirc);
> } else {
> printf(" %s%c\n",
> l_name,
> @@ -1032,9 +1032,9 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer,
> }
> if (doit) {
> if (dirc == ' ') {
> - printf(" %8ld %s%c\n",
> - (long)FAT2CPU32(dentptr->size),
> - s_name, dirc);
> + printf(" %8u %s%c\n",
> + FAT2CPU32(dentptr->size),
> + s_name, dirc);
> } else {
> printf(" %s%c\n",
> s_name, dirc);
> @@ -1153,20 +1153,28 @@ rootdir_done:
> }
>
> if (dogetsize)
> - ret = FAT2CPU32(dentptr->size);
> + *size = FAT2CPU32(dentptr->size);
> else
> - ret = get_contents(mydata, dentptr, pos, buffer, maxsize);
> - debug("Size: %d, got: %ld\n", FAT2CPU32(dentptr->size), ret);
> + *size = get_contents(mydata, dentptr, pos, buffer, maxsize);
> + debug("Size: %u, got: %u\n", FAT2CPU32(dentptr->size),
> + (unsigned) *size);
>
> exit:
> free(mydata->fatbuf);
> return ret;
> }
>
> -long
> +int
> do_fat_read(const char *filename, void *buffer, unsigned long maxsize, int dols)
> {
> - return do_fat_read_at(filename, 0, buffer, maxsize, dols, 0);
> + int ret;
> + off_t size;
> +
> + ret = do_fat_read_at(filename, 0, buffer, maxsize, dols, 0, &size);
> + if (ret == 0)
> + return size;
Will this cause problems if size is >2GB?
> + else
> + return ret;
> }
>
> int file_fat_detectfs(void)
> @@ -1238,21 +1246,34 @@ int file_fat_ls(const char *dir)
>
> int fat_exists(const char *filename)
> {
> - int sz;
> - sz = do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1);
> - return sz >= 0;
> + int ret;
> + off_t sz;
> +
> + ret = do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1, &sz);
> + return ret == 0;
> }
>
> -int fat_size(const char *filename)
> +int fat_size(const char *filename, off_t *sz)
> {
> - return do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1);
> + int ret;
> +
> + ret = do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1, sz);
> + return ret == 0;
Should this return an error?
> }
>
> long file_fat_read_at(const char *filename, unsigned long pos, void *buffer,
> unsigned long maxsize)
> {
> + int ret;
> + off_t sz;
> +
> printf("reading %s\n", filename);
> - return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO, 0);
> + ret = do_fat_read_at(filename, pos, buffer, maxsize, LS_NO, 0, &sz);
> +
> + if (ret == 0)
> + return sz;
> + else
> + return ret;
This is fine for now. It seems like in the future we should change the
fs interface to return an error code for every method.
> }
>
> long file_fat_read(const char *filename, void *buffer, unsigned long maxsize)
> diff --git a/fs/fs.c b/fs/fs.c
> index dd680f3..c2d5b5f 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -46,7 +46,7 @@ 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, off_t *sz)
> {
> return -1;
> }
> @@ -82,7 +82,7 @@ struct fstype_info {
> disk_partition_t *fs_partition);
> int (*ls)(const char *dirname);
> int (*exists)(const char *filename);
> - int (*size)(const char *filename);
> + int (*size)(const char *filename, off_t *size);
> 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);
> @@ -233,13 +233,13 @@ int fs_exists(const char *filename)
> return ret;
> }
>
> -int fs_size(const char *filename)
> +int fs_size(const char *filename, off_t *size)
> {
> int ret;
>
> struct fstype_info *info = fs_get_info(fs_type);
>
> - ret = info->size(filename);
> + ret = info->size(filename, size);
>
> fs_close();
>
> @@ -292,7 +292,8 @@ 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;
> + off_t size;
> + int ret;
>
> if (argc != 4)
> return CMD_RET_USAGE;
> @@ -300,8 +301,8 @@ 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)
> + ret = fs_size(argv[3], &size);
> + if (ret < 0)
> return CMD_RET_FAILURE;
>
> setenv_hex("filesize", size);
> diff --git a/fs/sandbox/sandboxfs.c b/fs/sandbox/sandboxfs.c
> index ba6402c..a384cc7 100644
> --- a/fs/sandbox/sandboxfs.c
> +++ b/fs/sandbox/sandboxfs.c
> @@ -27,8 +27,13 @@ long sandbox_fs_read_at(const char *filename, unsigned long pos,
> os_close(fd);
> return ret;
> }
> - if (!maxsize)
> - maxsize = os_get_filesize(filename);
> + if (!maxsize) {
> + ret = os_get_filesize(filename, (off_t *)&maxsize);
> + if (ret < 0) {
> + os_close(fd);
> + return ret;
> + }
> + }
> size = os_read(fd, buffer, maxsize);
> os_close(fd);
>
> @@ -74,15 +79,19 @@ int sandbox_fs_ls(const char *dirname)
>
> int sandbox_fs_exists(const char *filename)
> {
> - ssize_t sz;
> + off_t sz;
> + int ret;
>
> - sz = os_get_filesize(filename);
> - return sz >= 0;
> + ret = os_get_filesize(filename, &sz);
> + return ret == 0;
> }
>
> -int sandbox_fs_size(const char *filename)
> +int sandbox_fs_size(const char *filename, off_t *size)
> {
> - return os_get_filesize(filename);
> + int ret;
> +
> + ret = os_get_filesize(filename, size);
> + return ret == 0;
> }
>
> void sandbox_fs_close(void)
> diff --git a/include/ext4fs.h b/include/ext4fs.h
> index 6c419f3..1f04973 100644
> --- a/include/ext4fs.h
> +++ b/include/ext4fs.h
> @@ -129,14 +129,14 @@ int ext4fs_write(const char *fname, unsigned char *buffer,
> #endif
>
> struct ext_filesystem *get_fs(void);
> -int ext4fs_open(const char *filename);
> +int ext4fs_open(const char *filename, off_t *size);
> int ext4fs_read(char *buf, unsigned len);
> int ext4fs_mount(unsigned part_length);
> void ext4fs_close(void);
> void ext4fs_reinit_global(void);
> int ext4fs_ls(const char *dirname);
> int ext4fs_exists(const char *filename);
> -int ext4fs_size(const char *filename);
> +int ext4fs_size(const char *filename, off_t *len);
> 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);
> diff --git a/include/fat.h b/include/fat.h
> index 20ca3f3..56aa3b7 100644
> --- a/include/fat.h
> +++ b/include/fat.h
> @@ -198,7 +198,7 @@ int file_cd(const char *path);
> int file_fat_detectfs(void);
> int file_fat_ls(const char *dir);
> int fat_exists(const char *filename);
> -int fat_size(const char *filename);
> +int fat_size(const char *filename, off_t *len);
> 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);
> diff --git a/include/fs.h b/include/fs.h
> index 06a45f2..df39609 100644
> --- a/include/fs.h
> +++ b/include/fs.h
> @@ -55,7 +55,7 @@ int fs_exists(const char *filename);
> *
> * Returns the file's size in bytes, or a negative value if it doesn't exist.
As mentioned above I'm not sure your size is consistent.
> */
> -int fs_size(const char *filename);
> +int fs_size(const char *filename, off_t *len);
>
> /*
> * Read file "filename" from the partition previously set by fs_set_blk_dev(),
> diff --git a/include/os.h b/include/os.h
> index 0230a7f..75d9846 100644
> --- a/include/os.h
> +++ b/include/os.h
> @@ -219,7 +219,7 @@ const char *os_dirent_get_typename(enum os_dirent_t type);
> * @param fname Filename to check
> * @return size of file, or -1 if an error ocurred
> */
> -ssize_t os_get_filesize(const char *fname);
> +int os_get_filesize(const char *fname, off_t *size);
>
> /**
> * Write a character to the controlling OS terminal
> diff --git a/include/sandboxfs.h b/include/sandboxfs.h
> index e7c3262..8588a29 100644
> --- a/include/sandboxfs.h
> +++ b/include/sandboxfs.h
> @@ -26,7 +26,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 sandbox_fs_size(const char *filename);
> +int sandbox_fs_size(const char *filename, off_t *size);
> 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);
>
> --
> 1.9.1
>
Regards,
Simon
More information about the U-Boot
mailing list