| Message ID | b1b4190c519e445684331c3bea851a0e@rwthex-w2-b.rwth-ad.de |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
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
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);
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(-)