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

[Simon Glass]

Hi,

On 18 December 2014 at 07:40, Przemyslaw Marczak <p.marczak at samsung.com> wrote:
> Hello,
>
>
> On 12/18/2014 03:34 PM, Simon Glass wrote:
>>
>> Hi Przemyslaw,
>>
>> On 18 December 2014 at 07:32, Przemyslaw Marczak <p.marczak at samsung.com>
>> wrote:
>>>
>>> Hello,
>>>
>>> On 12/18/2014 02:47 PM, Simon Glass wrote:
>>>>
>>>>
>>>> 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?
>>>
>>>
>>>
>>> __u8 is used in a whole fat code, and also as a directory entry buffer,
>>> so
>>> why not to keep the whole code style?
>>
>>
>> OK, sounds good.
>>
>> Do you have any ideas on the bug I reported?
>>
>
> No, but I think that this is not any fat issue.

Can you explain what you mean here? What other software could be causing this?

Regards,
Simon