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

Hi Heinrich,

On Sun, 20 Nov 2022 at 20:47, Heinrich Schuchardt xypron.glpk@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@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