[U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue

[Simon Glass]

Hi,

On 11 December 2014 at 05:01, Przemyslaw Marczak <p.marczak at samsung.com> 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 <p.marczak at samsung.com>
> Cc: Mikhail Zolotaryov <lebon at lebon.org.ua>
> Cc: Tom Rini <trini at ti.com>
> Cc: Stephen Warren <swarren at nvidia.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Suriyan Ramasami <suriyan.r at gmail.com>
> Cc: Lukasz Majewski <l.majewski at samsung.com>
> Cc: Wolfgang Denk <wd at denx.de>
> ---
>  fs/fat/fat.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)

Tested-by: Simon Glass <sjg at chomium.org>

>
> 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.

> +       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