[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