[U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators

[Simon Glass]

Hi Rob,

On 3 September 2017 at 00:37, Rob Clark <robdclark at 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 <robdclark at 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.

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()?