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

[Rob Clark]

On Sat, Sep 9, 2017 at 12:55 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Rob,
>
> On 5 September 2017 at 03:54, Rob Clark <robdclark at gmail.com> wrote:
>> On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg at chromium.org> wrote:
>>> 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.
>>
>> 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.
>

I'm not a huge fan of mixing styles.. but what I'd propose instead is this:

It turns out fat_write.c needs a lot of work for SCT too (ability to
write files in subdirectories, and write files w/ !=0 offset), so that
we can actually see the test results.. and before converting write
paths to use the new directory iterator, I was considering
re-organizing the code a bit, ie. move directory traversal code to
fs/fat/dir.c or util.c, move most of include/fat.h into a private
header, dropping the #include "fat.c" hack, etc.  That would be a good
time for flag-day cleanups like untypedef'ifying and perhaps cleaning
up all the #define macros that implicitly depend on having a 'mydata'
var.

So I'd suggest to leave it as-is for now, and change things to 'struct
foo' as part of the re-org.

That leaves FS_DIR in the later readdir patch.. I could change that to
'struct fs_dir_stream'.  That makes it not match the posix API it is
modelled after as closely, but maybe that is ok.

BR,
-R