On 2/1/19 4:20 PM, Chee, Tien Fong wrote:
On Fri, 2019-02-01 at 09:19 +0100, Marek Vasut wrote:
On 2/1/19 9:11 AM, Chee, Tien Fong wrote:
On Thu, 2019-01-31 at 15:22 +0100, Marek Vasut wrote:
On 1/31/19 1:42 PM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Drop the statically allocated get_contents_vfatname_block and dynamically allocate a buffer only if required. This saves 64KiB of memory.
Signed-off-by: Stefan Agner stefan.ag...@toradex.com Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v2
- Removed the change for debug message.
- Set allocation based on actual required size instead of
default max cluster size
fs/fat/fat.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index ecfa255..347787e 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -306,9 +306,6 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size) * into 'buffer'. * Update the number of bytes read in *gotsize or return -1 on fatal errors. */ -__u8 get_contents_vfatname_block[MAX_CLUSTSIZE]
- __aligned(ARCH_DMA_MINALIGN);
static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos, __u8 *buffer, loff_t maxsize, loff_t *gotsize) { @@ -351,15 +348,25 @@ static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos, /* align to beginning of next cluster if any */ if (pos) {
__u8 *tmp_buffer;actsize = min(filesize, (loff_t)bytesperclust);
if (get_cluster(mydata, curclust,get_contents_vfatname_block,
tmp_buffer = malloc_cache_aligned(actsize);if (!tmp_buffer) {debug("Error: allocating buffer\n");return -ENOMEM;}if (get_cluster(mydata, curclust, tmp_buffer,(int)actsize) != 0) {
Is the cast of actsize needed ?
Okay, i would remove it.
printf("Error reading cluster\n");
free(tmp_buffer);return -1; } filesize -= actsize; actsize -= pos;
memcpy(buffer, get_contents_vfatname_block +pos, actsize);
memcpy(buffer, tmp_buffer + pos, actsize);Hmmm, tmp_buffer is actsize big , but you're memcpy-ing actsize here, so this would mean you memcpy data past the end of tmp_buffer if pos
0, no?
This wouldn't happen because the pos and actsize are reset based on beginning of current cluster instead of beginning of a file. So, the memcpy would start at pos based on beginning of current cluster until the end of current cluster, that means the size it copies is still within a cluster size.
So pos is always 0 ?
Nop, reset here means the pos offset would be adjusted accordingly when the base change from beginning of the file to beginning of the cluster where the pos resides in.
You know the driver getting the content based on cluster by cluster.
OK, reading the code again, after simplification we get:
tmp_buffer = malloc_cache_aligned(actsize); ... actsize -= pos; memcpy(buffer, tmp_buffer + pos, actsize);
so there shouldn't be any read past the end of buffer.