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..."
> 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[].
BR,
-R
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
