[PATCH v3 16/18] pxe: Refactor sysboot to have one helper

[Ramon Fried]

On Thu, Oct 14, 2021 at 9:51 PM Simon Glass <sjg at chromium.org> wrote:
>
> The only difference between the three helpers is the filesystem type.
> Factor this out and call the filesystem functions directly, instead of
> through the command-line interpreter. This allows the file size to be
> obtained directly, instead of via an environment variable.
>
> We cannot do the same thing with PXE's tftpboot since there is no API
> at present to obtain information about the file that was read. So there
> is no point in changing pxe_getfile_func to use a ulong for the address,
> for example.
>
> This is as far as the refactoring can go for the present.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> (no changes since v1)
>
>  cmd/sysboot.c | 94 ++++++++++++++++++++-------------------------------
>  1 file changed, 36 insertions(+), 58 deletions(-)
>
> diff --git a/cmd/sysboot.c b/cmd/sysboot.c
> index 6344ecd357b..04c07020269 100644
> --- a/cmd/sysboot.c
> +++ b/cmd/sysboot.c
> @@ -6,63 +6,40 @@
>  #include <fs.h>
>  #include <pxe_utils.h>
>
> -static char *fs_argv[5];
> -
> -static int do_get_ext2(struct pxe_context *ctx, const char *file_path,
> -                      char *file_addr, ulong *sizep)
> +/**
> + * struct sysboot_info - useful information for sysboot helpers
> + *
> + * @fstype: Filesystem type (FS_TYPE_...)
> + * @ifname: Interface name (e.g. "ide", "scsi")
> + * @dev_part_str is in the format:
> + *     <dev>.<hw_part>:<part> where <dev> is the device number,
> + *     <hw_part> is the optional hardware partition number and
> + *     <part> is the partition number
> + */
> +struct sysboot_info {
> +       int fstype;
> +       const char *ifname;
> +       const char *dev_part_str;
> +};
> +
> +static int sysboot_read_file(struct pxe_context *ctx, const char *file_path,
> +                            char *file_addr, ulong *sizep)
>  {
> -#ifdef CONFIG_CMD_EXT2
> +       struct sysboot_info *info = ctx->userdata;
> +       loff_t len_read;
> +       ulong addr;
>         int ret;
>
> -       fs_argv[0] = "ext2load";
> -       fs_argv[3] = file_addr;
> -       fs_argv[4] = (void *)file_path;
> -
> -       if (!do_ext2load(ctx->cmdtp, 0, 5, fs_argv))
> -               return 1;
> -       ret = pxe_get_file_size(sizep);
> +       addr = simple_strtoul(file_addr, NULL, 16);
> +       ret = fs_set_blk_dev(info->ifname, info->dev_part_str, info->fstype);
>         if (ret)
> -               return log_msg_ret("tftp", ret);
> -#endif
> -       return -ENOENT;
> -}
> -
> -static int do_get_fat(struct pxe_context *ctx, const char *file_path,
> -                     char *file_addr, ulong *sizep)
> -{
> -#ifdef CONFIG_CMD_FAT
> -       int ret;
> -
> -       fs_argv[0] = "fatload";
> -       fs_argv[3] = file_addr;
> -       fs_argv[4] = (void *)file_path;
> -
> -       if (!do_fat_fsload(ctx->cmdtp, 0, 5, fs_argv))
> -               return 1;
> -       ret = pxe_get_file_size(sizep);
> +               return ret;
> +       ret = fs_read(file_path, addr, 0, 0, &len_read);
>         if (ret)
> -               return log_msg_ret("tftp", ret);
> -#endif
> -       return -ENOENT;
> -}
> -
> -static int do_get_any(struct pxe_context *ctx, const char *file_path,
> -                     char *file_addr, ulong *sizep)
> -{
> -#ifdef CONFIG_CMD_FS_GENERIC
> -       int ret;
> -
> -       fs_argv[0] = "load";
> -       fs_argv[3] = file_addr;
> -       fs_argv[4] = (void *)file_path;
> +               return ret;
> +       *sizep = len_read;
>
> -       if (!do_load(ctx->cmdtp, 0, 5, fs_argv, FS_TYPE_ANY))
> -               return 1;
> -       ret = pxe_get_file_size(sizep);
> -       if (ret)
> -               return log_msg_ret("tftp", ret);
> -#endif
> -       return -ENOENT;
> +       return 0;
>  }
>
>  /*
> @@ -74,9 +51,9 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, int argc,
>                       char *const argv[])
>  {
>         unsigned long pxefile_addr_r;
> -       pxe_getfile_func getfile;
>         struct pxe_context ctx;
>         char *pxefile_addr_str;
> +       struct sysboot_info info;
>         char *filename;
>         int prompt = 0;
>         int ret;
> @@ -106,24 +83,25 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, int argc,
>         }
>
>         if (strstr(argv[3], "ext2")) {
> -               getfile = do_get_ext2;
> +               info.fstype = FS_TYPE_EXT;
>         } else if (strstr(argv[3], "fat")) {
> -               getfile = do_get_fat;
> +               info.fstype = FS_TYPE_FAT;
>         } else if (strstr(argv[3], "any")) {
> -               getfile = do_get_any;
> +               info.fstype = FS_TYPE_ANY;
>         } else {
>                 printf("Invalid filesystem: %s\n", argv[3]);
>                 return 1;
>         }
> -       fs_argv[1] = argv[1];
> -       fs_argv[2] = argv[2];
> +       info.ifname = argv[1];
> +       info.dev_part_str = argv[2];
>
>         if (strict_strtoul(pxefile_addr_str, 16, &pxefile_addr_r) < 0) {
>                 printf("Invalid pxefile address: %s\n", pxefile_addr_str);
>                 return 1;
>         }
>
> -       if (pxe_setup_ctx(&ctx, cmdtp, getfile, NULL, true, filename)) {
> +       if (pxe_setup_ctx(&ctx, cmdtp, sysboot_read_file, &info, true,
> +                         filename)) {
>                 printf("Out of memory\n");
>                 return CMD_RET_FAILURE;
>         }
> --
> 2.33.0.1079.g6e70778dc9-goog
>
Reviewed-by: Ramon Fried <rfried.dev at gmail.com>