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

[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 <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);
-- 
2.10.1

[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] [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