From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Bruens Date: Sun, 6 Nov 2016 18:12:58 +0100 Subject: [U-Boot] [PATCH 1/2] ext4: Allow reading files with non-zero offset, clamp read len In-Reply-To: <41684559-0411-4187-3b5e-0d770c3abc92@wwwdotorg.org> References: <41684559-0411-4187-3b5e-0d770c3abc92@wwwdotorg.org> Message-ID: <22068908.cRyZAGdtvO@pebbles.site> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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