From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Thu, 18 Dec 2014 15:06:42 +0100 Subject: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue In-Reply-To: References: <1418299263-1148-1-git-send-email-p.marczak@samsung.com> Message-ID: <5492DF72.5080203@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello, On 12/18/2014 02:47 PM, Simon Glass wrote: > Hi, > > On 11 December 2014 at 05:01, Przemyslaw Marczak wrote: >> The present fat implementation ignores FAT16 long name >> directory entries which aren't placed in a single sector. >> >> This was becouse of the buffer was always filled by the >> two sectors, and the loop was made also for two sectors. >> >> If some file long name entries are stored in two sectors, >> the we have two cases: >> >> Case 1: >> Both of sectors are in the buffer - all required data >> for long file name is in the buffer. >> - Read OK! >> >> Case 2: >> The current directory entry is placed at the end of the >> second buffered sector. And the next entries are placed >> in a sector which is not buffered yet. Then two next >> sectors are buffered and the mentioned entry is ignored. >> - Read fail! >> >> This commit fixes this issue by: >> - read two sectors after loop on each single is done >> - keep the last used sector as a first in the buffer >> before the read of two next >> >> The commit doesn't affects the fat32 imlementation, >> which works good as previous. >> >> Signed-off-by: Przemyslaw Marczak >> Cc: Mikhail Zolotaryov >> Cc: Tom Rini >> Cc: Stephen Warren >> Cc: Simon Glass >> Cc: Suriyan Ramasami >> Cc: Lukasz Majewski >> Cc: Wolfgang Denk >> --- >> fs/fat/fat.c | 33 ++++++++++++++++++++++++--------- >> 1 file changed, 24 insertions(+), 9 deletions(-) > > Tested-by: Simon Glass > Thank you. >> >> diff --git a/fs/fat/fat.c b/fs/fat/fat.c >> index 04a51db..afbf12d 100644 >> --- a/fs/fat/fat.c >> +++ b/fs/fat/fat.c >> @@ -823,8 +823,11 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer, >> int ret = -1; >> int firsttime; >> __u32 root_cluster = 0; >> + __u32 read_blk; >> int rootdir_size = 0; >> - int j; >> + int j, k; > > What is k? Can we use a proper variable name? Also for j. That might > save needing a comment for them. > k is a counter for a first time read. I will change this code and add some comment. >> + int do_read; >> + __u8 *dir_ptr; > > Why does it use __u8 instead of u8 or uint8_t for example? >> >> if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) { >> debug("Error: reading boot sector\n"); >> @@ -910,23 +913,35 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer, >> } >> >> j = 0; >> + k = 0; >> while (1) { >> int i; >> >> - if (j == 0) { >> + if (mydata->fatsize == 32 || !k) { >> + dir_ptr = do_fat_read_at_block; >> + k = 1; >> + } else { >> + dir_ptr = (do_fat_read_at_block + mydata->sect_size); >> + memcpy(do_fat_read_at_block, dir_ptr, mydata->sect_size); >> + } >> + >> + do_read = 1; >> + >> + if (mydata->fatsize == 32 && j) >> + do_read = 0; >> + >> + if (do_read) { >> debug("FAT read sect=%d, clust_size=%d, DIRENTSPERBLOCK=%zd\n", >> cursect, mydata->clust_size, DIRENTSPERBLOCK); >> >> - if (disk_read(cursect, >> - (mydata->fatsize == 32) ? >> - (mydata->clust_size) : >> - PREFETCH_BLOCKS, >> - do_fat_read_at_block) < 0) { >> + read_blk = (mydata->fatsize == 32) ? >> + mydata->clust_size : PREFETCH_BLOCKS; >> + if (disk_read(cursect, read_blk, dir_ptr) < 0) { >> debug("Error: reading rootdir block\n"); >> goto exit; >> } >> >> - dentptr = (dir_entry *) do_fat_read_at_block; >> + dentptr = (dir_entry *)dir_ptr; >> } >> >> for (i = 0; i < DIRENTSPERBLOCK; i++) { >> @@ -951,7 +966,7 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer, >> >> get_vfatname(mydata, >> root_cluster, >> - do_fat_read_at_block, >> + dir_ptr, >> dentptr, l_name); >> >> if (dols == LS_ROOT) { >> -- >> 1.9.1 >> > > Regards, > Simon > Thank you and best regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com