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

[Stefan Bruens]

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

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019
work: +49 2405 49936-424
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Stephen Warren]

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?

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Stefan Brüns]

Support was already implemented, but not hooked up. This fixes several
fails in the test cases.

Signed-off-by: Stefan Brüns 
---
 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, _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);
-- 
2.10.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot