Re: [PATCH 3/5] fs: ext4: implement opendir, readdir, closedir

Hi Heinrich,

On Sat, 26 Oct 2024 at 08:41, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:

For accessing directories from the EFI sub-system a file system must implement opendir, readdir, closedir. Provide the missing implementation.

With this patch the eficonfig command can be used to define load options for the ext4 file system.

Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

fs/ext4/ext4fs.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++- fs/fs.c | 4 +- include/ext4fs.h | 4 ++ 3 files changed, 166 insertions(+), 3 deletions(-)

with nits/questions below

Reviewed-by: Simon Glass sjg@chromium.org

diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 21714149ef5..32693198aeb 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -21,17 +21,36 @@ */

#include <blk.h> +#include <div64.h> +#include <errno.h> #include <ext_common.h> #include <ext4fs.h> -#include "ext4_common.h" -#include <div64.h> #include <malloc.h> #include <part.h> #include <u-boot/uuid.h> +#include "ext4_common.h"

int ext4fs_symlinknest; struct ext_filesystem ext_fs;

+/**

• • struct ext4_dir_stream - ext4 directory stream
• • @parent: partition data used by fs layer.
• • This field must be at the beginning of the structure.
• • All other fields are private to the ext4 driver.
• • @root: root directory node
• • @dir: directory node
• • @dirent: directory stream entry
• • @fpos: file position in directory
• */

+struct ext4_dir_stream {

•   struct fs_dir_stream parent;
  

•   char *dirname;
  

•   struct fs_dirent dirent;
  

•   unsigned int fpos;
  

+};

struct ext_filesystem *get_fs(void) { return &ext_fs; @@ -205,6 +224,144 @@ int ext4fs_ls(const char *dirname) return 0; }

+int ext4fs_opendir(const char *dirname, struct fs_dir_stream **dirsp) +{

•   struct ext4_dir_stream *dirs;
  

•   struct ext2fs_node *dir = NULL;
  

•   int ret;
  

•   *dirsp = NULL;
  

•   dirs = calloc(1, sizeof(struct ext4_dir_stream));
  

•   if (!dirs)
  

•           return -ENOMEM;
  

•   dirs->dirname = strdup(dirname);
  

•   if (!dirs) {
  

•           free(dirs);
  

•           return -ENOMEM;
  

•   }
  

•   ret = ext4fs_find_file(dirname, &ext4fs_root->diropen, &dir,
  

•                          FILETYPE_DIRECTORY);
  

•   if (ret == 1) {
  

•           ret = 0;
  

•           *dirsp = (struct fs_dir_stream *)dirs;
  

•   } else {
  

•           ret = -ENOENT;
  

•   }
  

•   if (dir)
  

•           ext4fs_free_node(dir, &ext4fs_root->diropen);
  

•   return ret;
  

+}

+int ext4fs_readdir(struct fs_dir_stream *fs_dirs, struct fs_dirent **dentp) +{

•   struct ext4_dir_stream *dirs = (struct ext4_dir_stream *)fs_dirs;
  

•   struct fs_dirent *dent = &dirs->dirent;
  

•   struct ext2fs_node *dir = NULL;
  

•   int ret;
  

•   loff_t actread;
  

•   struct ext2fs_node fdiro;
  

•   int len;
  

•   struct ext2_dirent dirent;
  

•   *dentp = NULL;
  

•   ret = ext4fs_find_file(dirs->dirname, &ext4fs_root->diropen, &dir,
  

•                          FILETYPE_DIRECTORY);
  

•   if (ret != 1) {
  

•           ret = -ENOENT;
  

•           goto out;
  

•   }
  

•   if (!dir->inode_read) {
  

•           ret = ext4fs_read_inode(dir->data, dir->ino, &dir->inode);
  

•           if (!ret) {
  

•                   ret = -EIO;
  

•                   goto out;
  

•           }
  

•   }
  

•   if (dirs->fpos >= le32_to_cpu(dir->inode.size))
  

•           return -ENOENT;
  

•   memset(dent, 0, sizeof(struct fs_dirent));
  

•   while (dirs->fpos < le32_to_cpu(dir->inode.size)) {
  

•           ret = ext4fs_read_file(dir, dirs->fpos,
  

•                                  sizeof(struct ext2_dirent),
  

•                                  (char *)&dirent, &actread);
  

•           if (ret < 0)
  

•                   return -ret;
  

•           if (!dirent.direntlen)
  

•                   return -EIO;
  

Should this (and the return immediately above) free call ext4fs_free_node() ?

•           if (dirent.namelen)
  

•                   break;
  

•           dirs->fpos += le16_to_cpu(dirent.direntlen);
  

•   }
  

•   len = min(FS_DIRENT_NAME_LEN - 1, (int)dirent.namelen);
  

•   ret = ext4fs_read_file(dir, dirs->fpos + sizeof(struct ext2_dirent),
  

•                          len, dent->name, &actread);
  

•   if (ret < 0)
  

•           goto out;
  

•   dent->name[len] = '\0';
  

•   fdiro.data = dir->data;
  

•   fdiro.ino = le32_to_cpu(dirent.inode);
  

•   ret = ext4fs_read_inode(dir->data, fdiro.ino, &fdiro.inode);
  

•   if (!ret) {
  

•           ret = -EIO;
  

•           goto out;
  

•   }
  

•   switch (le16_to_cpu(fdiro.inode.mode) & FILETYPE_INO_MASK) {
  

•   case FILETYPE_INO_DIRECTORY:
  

•           dent->type = FS_DT_DIR;
  

•           break;
  

•   case FILETYPE_INO_SYMLINK:
  

•           dent->type = FS_DT_LNK;
  

•           break;
  

•   case FILETYPE_INO_REG:
  

•           dent->type = FS_DT_REG;
  

•           break;
  

•   default:
  

•           dent->type = FILETYPE_UNKNOWN;
  

•   }
  

•   rtc_to_tm(fdiro.inode.atime, &dent->access_time);
  

•   rtc_to_tm(fdiro.inode.ctime, &dent->create_time);
  

•   rtc_to_tm(fdiro.inode.mtime, &dent->change_time);
  

•   dirs->fpos += le16_to_cpu(dirent.direntlen);
  

•   dent->size = fdiro.inode.size;
  

•   *dentp = dent;
  

•   ret = 0;
  

+out:

•   if (dir)
  

•           ext4fs_free_node(dir, &ext4fs_root->diropen);
  

•   return ret;
  

+}

+void ext4fs_closedir(struct fs_dir_stream *fs_dirs) +{

•   struct ext4_dir_stream *dirs = (struct ext4_dir_stream *)fs_dirs;
  

•   if (!dirs)
  

•           return;
  

•   free(dirs->dirname);
  

•   free(dirs);
  

+}

int ext4fs_exists(const char *filename) { struct ext2fs_node *dirnode = NULL; diff --git a/fs/fs.c b/fs/fs.c index e2915e7cf79..a515905c4c9 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -232,7 +232,9 @@ static struct fstype_info fstypes[] = { .ln = fs_ln_unsupported, #endif .uuid = ext4fs_uuid,

•           .opendir = fs_opendir_unsupported,
  

•           .opendir = ext4fs_opendir,
  

•           .readdir = ext4fs_readdir,
  

•           .closedir = ext4fs_closedir,
            .unlink = fs_unlink_unsupported,
            .mkdir = fs_mkdir_unsupported,
    },
  

diff --git a/include/ext4fs.h b/include/ext4fs.h index 41f9eb8bd33..fe3fb301ec8 100644 --- a/include/ext4fs.h +++ b/include/ext4fs.h @@ -27,6 +27,7 @@ #ifndef __EXT4__ #define __EXT4__ #include <ext_common.h> +#include <fs.h>

struct disk_partition;

@@ -218,4 +219,7 @@ int ext4fs_uuid(char *uuid_str); void ext_cache_init(struct ext_block_cache *cache); void ext_cache_fini(struct ext_block_cache *cache); int ext_cache_read(struct ext_block_cache *cache, lbaint_t block, int size); +int ext4fs_opendir(const char *dirname, struct fs_dir_stream **dirsp); +int ext4fs_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp); +void ext4fs_closedir(struct fs_dir_stream *dirs);

Please add function comments

#endif

2.45.2

Regards, Simon