On Sat, Sep 9, 2017 at 12:55 AM, Simon Glass <s...@chromium.org> wrote:
> 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.
>
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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
