[U-Boot] [PATCH V2 5/5] fat: implement exists() for FAT fs

[Stephen Warren <swarren@wwwdotorg.org>]

On 01/26/2014 08:52 AM, Simon Glass wrote:
> Hi Stephen,
> 
> On 23 January 2014 12:57, Stephen Warren <swarren@wwwdotorg.org
> <mailto:swarren@wwwdotorg.org>> wrote:
> 
>     From: Stephen Warren <swarren at nvidia.com <mailto:swarren@nvidia.com>>
> 
>     This hooks into the generic "file exists" support added in an earlier
>     patch, and provides an implementation for the ext4 filesystem.

>     diff --git a/fs/fat/fat.c b/fs/fat/fat.c

>     @@ -808,7 +808,7 @@ __u8 do_fat_read_at_block[MAX_CLUSTSIZE]
> 
>      long
>      do_fat_read_at(const char *filename, unsigned long pos, void *buffer,
>     -              unsigned long maxsize, int dols)
>     +              unsigned long maxsize, int dols, int dogetsize)
> 
> I think it would be better to combine the three available operations
> (read, ls and getsize) into an enum and pass the required operation
> explicitly into this function in a single parameter.

I had originally thought that, but the implementation of the function
changes the value of dols during operation. I suppose it would be
possible to achieve that using bit-mask operations on the variable, but
it seemed a bit cleaner to just make it a separate variable rather than
using fields within a somewhat unrelated variable.

Would you still prefer to combine them into one:?

>     -       ret = get_contents(mydata, dentptr, pos, buffer, maxsize);
>     +       if (dogetsize)
>     +               ret = FAT2CPU32(dentptr->size);
>     +       else
>     +              
> 
> 
> Doesn't this mean you are actually reading the contents here into a NULL
> buffer? At least I think it needs a comment as to why this works.
> 
>     ret = get_contents(mydata, dentptr, pos, buffer, maxsize);
>             debug("Size: %d, got: %ld\n", FAT2CPU32(dentptr->size), ret);

get_contents() is the function that actually reads the file content;
everything before this point is simply parsing the directory entry for
the file. So, no, I don't think the file content is read at all.

That implementation of dols!=0 is somewhat similar, although it does
"goto exit" at a different location in the code.