[PATCH v10 4/7] eficonfig: expose eficonfig_create_device_path()

[Masahisa Kojima]

Hi Heinrich,

On Sun, 20 Nov 2022 at 20:47, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 11/20/22 01:21, Masahisa Kojima wrote:
> > Following commits are adding support for UEFI variable management
> > via the eficonfig menu. Those functions needs to use
> > eficonfig_create_device_path() to construct the full device path
> > from device path of the volume and file path, so move it
> > out of their static declarations.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > ---
> > Newly created in v10
> >
> >   cmd/eficonfig.c      | 20 +++++++++++---------
> >   include/efi_config.h |  2 ++
> >   2 files changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> > index 12babb76c2..58a6856666 100644
> > --- a/cmd/eficonfig.c
> > +++ b/cmd/eficonfig.c
> > @@ -436,14 +436,15 @@ static efi_status_t eficonfig_volume_selected(void *data)
> >   }
> >
> >   /**
> > - * create_selected_device_path() - create device path
> > + * eficonfig_create_device_path() - create device path
> >    *
> > - * @file_info:       pointer to the selected file information
> > + * @dp_volume:       pointer to the volume
> > + * @current_path: pointer to the file path u16 string
> >    * Return:
> >    * device path or NULL. Caller must free the returned value
> >    */
> > -static
> > -struct efi_device_path *create_selected_device_path(struct eficonfig_select_file_info *file_info)
> > +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume,
> > +                                                  u16 *current_path)
> >   {
> >       char *p;
> >       void *buf;
> > @@ -452,7 +453,7 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file
> >       struct efi_device_path_file_path *fp;
>
> This function should be moved to lib/efi_loader/efi_device_path.c so
> that we can use it to simplify efi_dp_from_file().

eficonfig_create_device_path() takes u16 file path string, but
efi_dp_from_file() takes
file path string, so additional charset conversion is required to call
this library function
from efi_dp_from_file() and I wonder it simplifies the code.

>
> >
> >       fp_size = sizeof(struct efi_device_path) +
> > -               ((u16_strlen(file_info->current_path) + 1) * sizeof(u16));
> > +               ((u16_strlen(current_path) + 1) * sizeof(u16));
>
> You could reduce the code size a little with:
>
> fp_size = sizeof(struct efi_device_path) +
> u16_strsize(file_info->current_path);

I'm aware that you have already merged this series.
I will send a follow up patch to fix this u16_strsize().

Thanks,
Masahisa Kojima


>
> Best regards
>
> Heinrich
>
> >       buf = calloc(1, fp_size + sizeof(END));
> >       if (!buf)
> >               return NULL;
> > @@ -461,13 +462,13 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file
> >       fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE,
> >       fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH,
> >       fp->dp.length = (u16)fp_size;
> > -     u16_strcpy(fp->str, file_info->current_path);
> > +     u16_strcpy(fp->str, current_path);
> >
> >       p = buf;
> >       p += fp_size;
> >       *((struct efi_device_path *)p) = END;
> >
> > -     dp = efi_dp_append(file_info->dp_volume, (struct efi_device_path *)buf);
> > +     dp = efi_dp_append(dp_volume, (struct efi_device_path *)buf);
> >       free(buf);
> >
> >       return dp;
> > @@ -1472,7 +1473,8 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
> >       }
> >
> >       if (bo->initrd_info.dp_volume) {
> > -             dp = create_selected_device_path(&bo->initrd_info);
> > +             dp = eficonfig_create_device_path(bo->initrd_info.dp_volume,
> > +                                              bo->initrd_info.current_path);
> >               if (!dp) {
> >                       ret = EFI_OUT_OF_RESOURCES;
> >                       goto out;
> > @@ -1481,7 +1483,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
> >               efi_free_pool(dp);
> >       }
> >
> > -     dp = create_selected_device_path(&bo->file_info);
> > +     dp = eficonfig_create_device_path(bo->file_info.dp_volume, bo->file_info.current_path);
> >       if (!dp) {
> >               ret = EFI_OUT_OF_RESOURCES;
> >               goto out;
> > diff --git a/include/efi_config.h b/include/efi_config.h
> > index 064f2efe3f..934de41e85 100644
> > --- a/include/efi_config.h
> > +++ b/include/efi_config.h
> > @@ -99,5 +99,7 @@ efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu,
> >                                        char *title, eficonfig_entry_func func,
> >                                        void *data);
> >   efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu);
> > +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume,
> > +                                                  u16 *current_path);
> >
> >   #endif
>