Hi Rob,
On 5 September 2017 at 03:54, Rob Clark <robdcl...@gmail.com> wrote:
> On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <s...@chromium.org> wrote:
>> Hi Rob,
>>
>> On 3 September 2017 at 00:37, Rob Clark <robdcl...@gmail.com> wrote:
>>> Untangle directory traversal into a simple iterator, to replace the
>>> existing multi-purpose do_fat_read_at() + get_dentfromdir().
>>>
>>> Signed-off-by: Rob Clark <robdcl...@gmail.com>
>>> ---
>>> fs/fat/fat.c | 326
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 326 insertions(+)
>>>
>>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>>> index e1c0a15dc7..c72d6ca931 100644
>>> --- a/fs/fat/fat.c
>>> +++ b/fs/fat/fat.c
>>> @@ -1245,6 +1245,332 @@ exit:
>>> return ret;
>>> }
>>>
>>> +
>>> +/*
>>> + * Directory iterator, to simplify filesystem traversal
>>> + */
>>> +
>>> +typedef struct {
>>
>> Please avoid using a typedef here. It seems like it could just be a struct.
>
> I'm typically not a fan of 'typedef struct' but that seems to be the
> style that fs/fat code was already using, and "when in Rome..."
Sure, but let's move to Venice as it is less dusty.
>
>> Also pleaee add a comment about what the struct is for
>>
>>> + fsdata *fsdata;
>>> + unsigned cursect;
>>> + dir_entry *dent;
>>> + int remaining; /* remaining dent's in current cluster */
>>> + int last_cluster;
>>> + int is_root;
>>> +
>>> + /* current iterator position values: */
>>> + char l_name[VFAT_MAXLEN_BYTES];
>>> + char s_name[14];
>>> + char *name; /* l_name if there is one, else s_name */
>>> +
>>> + u8 block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
>>
>> Some members are missing comments.
>>
>> Also I'm not too sure how the alignment works here. I don't see you
>> creating one of these objects in this patch, but could you add a
>> comment to the struct about how to create one? Do you use memalign()?
>
> I was just creating them on the stack normally.. expecting the
> compiler to dtrt because of the __aligned() on block[].
Well I think that works. OK.
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
