[U-Boot] [PATCH 1/2] ext4: Allow reading files with non-zero offset, clamp read len

Support was already implemented, but not hooked up. This fixes several fails in the test cases.
Signed-off-by: Stefan Brüns stefan.bruens@rwth-aachen.de --- fs/ext4/ext4fs.c | 15 +++++---------- include/ext4fs.h | 2 +- 2 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 3078737..f8cf6cd 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -190,12 +190,12 @@ int ext4fs_size(const char *filename, loff_t *size) return ext4fs_open(filename, size); }
-int ext4fs_read(char *buf, loff_t len, loff_t *actread) +int ext4fs_read(char *buf, loff_t offset, loff_t len, loff_t *actread) { if (ext4fs_root == NULL || ext4fs_file == NULL) return 0;
- return ext4fs_read_file(ext4fs_file, 0, len, buf, actread); + return ext4fs_read_file(ext4fs_file, offset, len, buf, actread); }
int ext4fs_probe(struct blk_desc *fs_dev_desc, @@ -217,21 +217,16 @@ int ext4_read_file(const char *filename, void *buf, loff_t offset, loff_t len, loff_t file_len; int ret;
- if (offset != 0) { - printf("** Cannot support non-zero offset **\n"); - return -1; - } - ret = ext4fs_open(filename, &file_len); if (ret < 0) { printf("** File not found %s **\n", filename); return -1; }
- if (len == 0) - len = file_len; + if ((len == 0) || (offset + len > file_len)) + len = (file_len - offset);
- return ext4fs_read(buf, len, len_read); + return ext4fs_read(buf, offset, len, len_read); }
int ext4fs_uuid(char *uuid_str) diff --git a/include/ext4fs.h b/include/ext4fs.h index 965cd9e..bb55639 100644 --- a/include/ext4fs.h +++ b/include/ext4fs.h @@ -135,7 +135,7 @@ int ext4_write_file(const char *filename, void *buf, loff_t offset, loff_t len,
struct ext_filesystem *get_fs(void); int ext4fs_open(const char *filename, loff_t *len); -int ext4fs_read(char *buf, loff_t len, loff_t *actread); +int ext4fs_read(char *buf, loff_t offset, loff_t len, loff_t *actread); int ext4fs_mount(unsigned part_length); void ext4fs_close(void); void ext4fs_reinit_global(void);

On 11/05/2016 06:32 PM, Stefan Brüns wrote:
Support was already implemented, but not hooked up. This fixes several fails in the test cases.
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c
@@ -217,21 +217,16 @@ int ext4_read_file(const char *filename, void *buf, loff_t offset, loff_t len,
- if (len == 0)
len = file_len;
- if ((len == 0) || (offset + len > file_len))
len = (file_len - offset);
Isn't (offset + len > file_len) an error? It seems find to "read to EOF" if the caller specified len==0, but if they specified a specific len, then isn't it an error if len+offset exceeds the length of the file?
On the other hand, if this is how other filesystems work in U-Boot, it's fine. I suppose this is consistent with how POSIX read() works.
diff --git a/include/ext4fs.h b/include/ext4fs.h
-int ext4fs_read(char *buf, loff_t len, loff_t *actread); +int ext4fs_read(char *buf, loff_t offset, loff_t len, loff_t *actread);
Don't you need to update all callers of this function in this patch so the build doesn't break?

On Samstag, 5. November 2016 21:22:43 CET Stephen Warren wrote:
On 11/05/2016 06:32 PM, Stefan Brüns wrote:
Support was already implemented, but not hooked up. This fixes several fails in the test cases.
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c
@@ -217,21 +217,16 @@ int ext4_read_file(const char *filename, void *buf, loff_t offset, loff_t len,
- if (len == 0)
len = file_len;
- if ((len == 0) || (offset + len > file_len))
len = (file_len - offset);
Isn't (offset + len > file_len) an error? It seems find to "read to EOF" if the caller specified len==0, but if they specified a specific len, then isn't it an error if len+offset exceeds the length of the file?
On the other hand, if this is how other filesystems work in U-Boot, it's fine. I suppose this is consistent with how POSIX read() works.
It matches behaviour of POSIX read() and u-boot's FAT implementation, and there is also the actread parameter the caller could/should check.
diff --git a/include/ext4fs.h b/include/ext4fs.h
-int ext4fs_read(char *buf, loff_t len, loff_t *actread); +int ext4fs_read(char *buf, loff_t offset, loff_t len, loff_t *actread);
Don't you need to update all callers of this function in this patch so the build doesn't break?
Missed the calls from spl_ext.c, will send v2.
Kind regards,
Stefan
participants (3)
-
Stefan Bruens
-
Stefan Brüns
-
Stephen Warren