[PATCH v3 1/7] efi_loader: Refactor device and image paths into a function

[Ilias Apalodimas]

On Sat, 21 Dec 2024 at 10:21, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Simon,
> I don't see any of the requested changes from v2 on this one.

Ah you changed it to static instead. That's fine we can move it to the
dp helpers. if someone else needs it.

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>

>
> Cheers
> /Ilias
>
> On Thu, 19 Dec 2024 at 04:39, Simon Glass <sjg at chromium.org> wrote:
> >
> > Move this code into a function so it can be called from elsewhere.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v3:
> > - Make calculate_paths() static and add a comment
> >
> >  lib/efi_loader/efi_bootbin.c | 85 ++++++++++++++++++++++++------------
> >  1 file changed, 57 insertions(+), 28 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c
> > index b677bbc3124..5016ba7e225 100644
> > --- a/lib/efi_loader/efi_bootbin.c
> > +++ b/lib/efi_loader/efi_bootbin.c
> > @@ -44,12 +44,64 @@ void efi_clear_bootdev(void)
> >         image_size = 0;
> >  }
> >
> > +/**
> > + * calculate_paths() - Calculate the device and image patch given a device
> > + *
> > + * @dev:               device, e.g. "MMC"
> > + * @devnr:             number of the device, e.g. "1:2"
> > + * @path:              path to file loaded
> > + * @device_pathp:      returns EFI device path
> > + * @image_pathp:       returns EFI image path
> > + * Return EFI_SUCCESS on success, else error code
> > + */
> > +static efi_status_t calculate_paths(const char *dev, const char *devnr,
> > +                                   const char *path,
> > +                                   struct efi_device_path **device_pathp,
> > +                                   struct efi_device_path **image_pathp)
> > +{
> > +       struct efi_device_path *image, *device;
> > +       efi_status_t ret;
> > +
> > +#if IS_ENABLED(CONFIG_NETDEVICES)
> > +       if (!strcmp(dev, "Net") || !strcmp(dev, "Http")) {
> > +               ret = efi_net_set_dp(dev, devnr);
> > +               if (ret != EFI_SUCCESS)
> > +                       return ret;
> > +       }
> > +#endif
> > +
> > +       ret = efi_dp_from_name(dev, devnr, path, &device, &image);
> > +       if (ret != EFI_SUCCESS)
> > +               return ret;
> > +
> > +       *device_pathp = device;
> > +       if (image) {
> > +               /* FIXME: image should not contain device */
> > +               struct efi_device_path *image_tmp = image;
> > +
> > +               efi_dp_split_file_path(image, &device, &image);
> > +               efi_free_pool(image_tmp);
> > +       }
> > +       *image_pathp = image;
> > +       log_debug("- boot device %pD\n", device);
> > +       if (image)
> > +               log_debug("- image %pD\n", image);
> > +
> > +       return EFI_SUCCESS;
> > +}
> > +
> >  /**
> >   * efi_set_bootdev() - set boot device
> >   *
> >   * This function is called when a file is loaded, e.g. via the 'load' command.
> >   * We use the path to this file to inform the UEFI binary about the boot device.
> >   *
> > + * For a valid image, it sets:
> > + *    - image_addr to the provided buffer
> > + *    - image_size to the provided buffer_size
> > + *    - bootefi_device_path to the EFI device-path
> > + *    - bootefi_image_path to the EFI image-path
> > + *
> >   * @dev:               device, e.g. "MMC"
> >   * @devnr:             number of the device, e.g. "1:2"
> >   * @path:              path to file loaded
> > @@ -59,7 +111,6 @@ void efi_clear_bootdev(void)
> >  void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
> >                      void *buffer, size_t buffer_size)
> >  {
> > -       struct efi_device_path *device, *image;
> >         efi_status_t ret;
> >
> >         log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev,
> > @@ -93,34 +144,12 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
> >         image_addr = buffer;
> >         image_size = buffer_size;
> >
> > -#if IS_ENABLED(CONFIG_NETDEVICES)
> > -       if (!strcmp(dev, "Net") || !strcmp(dev, "Http")) {
> > -               ret = efi_net_set_dp(dev, devnr);
> > -               if (ret != EFI_SUCCESS)
> > -                       goto error;
> > -       }
> > -#endif
> > -
> > -       ret = efi_dp_from_name(dev, devnr, path, &device, &image);
> > -       if (ret != EFI_SUCCESS)
> > -               goto error;
> > -
> > -       bootefi_device_path = device;
> > -       if (image) {
> > -               /* FIXME: image should not contain device */
> > -               struct efi_device_path *image_tmp = image;
> > -
> > -               efi_dp_split_file_path(image, &device, &image);
> > -               efi_free_pool(image_tmp);
> > +       ret = calculate_paths(dev, devnr, path, &bootefi_device_path,
> > +                             &bootefi_image_path);
> > +       if (ret) {
> > +               log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
> > +               efi_clear_bootdev();
> >         }
> > -       bootefi_image_path = image;
> > -       log_debug("- boot device %pD\n", device);
> > -       if (image)
> > -               log_debug("- image %pD\n", image);
> > -       return;
> > -error:
> > -       log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
> > -       efi_clear_bootdev();
> >  }
> >
> >  /**
> > --
> > 2.34.1
> >