[U-Boot] [PATCH] fs: add fs_readdir()

BrĂ¼ns, Stefan Stefan.Bruens at rwth-aachen.de
Thu Aug 3 19:10:04 UTC 2017


On Donnerstag, 3. August 2017 18:54:30 CEST Rob Clark wrote:
> Needed to support efi file protocol.  The fallback.efi loader wants
> to be able to read the contents of the /EFI directory to find an OS
> to boot.
> 
> Currently only implemented for FAT, but that is all that UEFI is
> required to support.
> 
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
>  fs/fat/fat.c  | 59
> ++++++++++++++++++++++++++++++++++++++++++++++------------- fs/fs.c       |
> 25 +++++++++++++++++++++++++
>  include/fat.h |  4 +++-
>  include/fs.h  | 21 +++++++++++++++++++++
>  4 files changed, 95 insertions(+), 14 deletions(-)
> 

NAK

1. The current code is already much to convoluted. Your changes add to this 
significantly

2. Your patch description neither references the exact part of the EFI 
specification you want to support (which took me some time, for reference it 
is "13.: Protocols - Media Access, 13.5: File Protocol"), nor are you 
specifying the required semantics (which is "open", "read", "close", where 
each read returns a single directory entry, similar to the POSIX opendir(), 
readdir() calls. 

3. Usage of an index too lookup the next entry is also quite convoluted.

4. As far as I can see, your code will fail to find files in the root 
directory (look for LS_ROOT).

I think it would be much better to first restructure the current code to use 
an readdir like interface internally, and then do everything EFI needs on top.

This would get rid of the 4 almost identical copies to print the current 
directory entry (dols == LS_ROOT || dols == LS_YES), 2 copies of the remaining 
directory traversal and and also avoid the bug in (4.).

Kind regards,

Stefan


More information about the U-Boot mailing list