Re: [PATCH v2 1/2] eficonfig: carve out efi_get_next_variable_name_int calls

On Mon, Dec 19, 2022 at 11:33:12AM +0900, Masahisa Kojima wrote:

To retrieve the EFI variable name by efi_get_next_variable_name_int(), the sequence of alloc -> efi_get_next_variable_name_int -> realloc -> efi_get_next_variable_name_int is required. In current code, this sequence repeatedly appears in the several functions. It should be curved out a common function.

This commit also fixes the missing free() of var_name16 in eficonfig_delete_invalid_boot_option().

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

Changes in v2:

• fix typo in the commit message
• rename efi_get_variable_name to efi_next_variable_name

cmd/eficonfig.c | 62 +++++++++---------------------------- include/efi_loader.h | 2 ++ lib/efi_loader/efi_helper.c | 34 ++++++++++++++++++++ 3 files changed, 51 insertions(+), 47 deletions(-)

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c index 394ae67cce..0b07dfc958 100644 --- a/cmd/eficonfig.c +++ b/cmd/eficonfig.c @@ -1683,7 +1683,7 @@ static efi_status_t eficonfig_show_boot_selection(unsigned int *selected) u32 i; u16 *bootorder; efi_status_t ret;

• u16 *var_name16 = NULL, *p;

• u16 *var_name16 = NULL; efi_uintn_t num, size, buf_size; struct efimenu *efi_menu; struct list_head *pos, *n;

@@ -1718,24 +1718,12 @@ static efi_status_t eficonfig_show_boot_selection(unsigned int *selected) int index; efi_guid_t guid;

• size = buf_size;
  

• ret = efi_get_next_variable_name_int(&size, var_name16, &guid);
  

• ret = efi_next_variable_name(&buf_size, &var_name16, &guid);
  

  if (ret == EFI_NOT_FOUND) break;

• if (ret == EFI_BUFFER_TOO_SMALL) {
  

• 	buf_size = size;
  

• 	p = realloc(var_name16, buf_size);
  

• 	if (!p) {
  

• 		free(var_name16);
  

• 		return EFI_OUT_OF_RESOURCES;
  

• 	}
  

• 	var_name16 = p;
  

• 	ret = efi_get_next_variable_name_int(&size, var_name16, &guid);
  

• }
  

• if (ret != EFI_SUCCESS) {
  

• 	free(var_name16);
  

• 	return ret;
  

• }
  

• if (ret != EFI_SUCCESS)
  

• 	goto out;
  

• if (efi_varname_is_load_option(var_name16, &index)) { /* If the index is included in the BootOrder, skip it */ if (search_bootorder(bootorder, num, index, NULL))

@@ -2026,7 +2014,7 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi u32 i; char *title; efi_status_t ret;

• u16 *var_name16 = NULL, *p;

• u16 *var_name16 = NULL; efi_uintn_t size, buf_size;

  /* list the load option in the order of BootOrder variable */

@@ -2054,19 +2042,9 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi break;

size = buf_size;

• ret = efi_get_next_variable_name_int(&size, var_name16, &guid);
  

• ret = efi_next_variable_name(&buf_size, &var_name16, &guid);
  

  if (ret == EFI_NOT_FOUND) break;

• if (ret == EFI_BUFFER_TOO_SMALL) {
  

• 	buf_size = size;
  

• 	p = realloc(var_name16, buf_size);
  

• 	if (!p) {
  

• 		ret = EFI_OUT_OF_RESOURCES;
  

• 		goto out;
  

• 	}
  

• 	var_name16 = p;
  

• 	ret = efi_get_next_variable_name_int(&size, var_name16, &guid);
  

• }
  

  if (ret != EFI_SUCCESS) goto out;

@@ -2336,10 +2314,10 @@ efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_op efi_uintn_t size; void *load_option; struct efi_load_option lo;

• u16 *var_name16 = NULL, *p;

• u16 *var_name16 = NULL; u16 varname[] = u"Boot####"; efi_status_t ret = EFI_SUCCESS;

• efi_uintn_t varname_size, buf_size;

• efi_uintn_t buf_size;

  buf_size = 128; var_name16 = malloc(buf_size);

@@ -2352,24 +2330,12 @@ efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_op efi_guid_t guid; efi_uintn_t tmp;

• varname_size = buf_size;
  

• ret = efi_get_next_variable_name_int(&varname_size, var_name16, &guid);
  

• ret = efi_next_variable_name(&buf_size, &var_name16, &guid);
  

  if (ret == EFI_NOT_FOUND) break;

• if (ret == EFI_BUFFER_TOO_SMALL) {
  

• 	buf_size = varname_size;
  

• 	p = realloc(var_name16, buf_size);
  

• 	if (!p) {
  

• 		free(var_name16);
  

• 		return EFI_OUT_OF_RESOURCES;
  

• 	}
  

• 	var_name16 = p;
  

• 	ret = efi_get_next_variable_name_int(&varname_size, var_name16, &guid);
  

• }
  

• if (ret != EFI_SUCCESS) {
  

• 	free(var_name16);
  

• 	return ret;
  

• }
  

• if (ret != EFI_SUCCESS)
  

• 	goto out;
  

• if (!efi_varname_is_load_option(var_name16, &index)) continue;

@@ -2407,6 +2373,8 @@ next: }

out:

• free(var_name16);
• return ret;

}

diff --git a/include/efi_loader.h b/include/efi_loader.h index 0899e293e5..699176872d 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -708,6 +708,8 @@ int algo_to_len(const char *algo); int efi_link_dev(efi_handle_t handle, struct udevice *dev); int efi_unlink_dev(efi_handle_t handle); bool efi_varname_is_load_option(u16 *var_name16, int *index); +efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf,

• 		    efi_guid_t *guid);
  
  

/**

• efi_size_in_pages() - convert size in bytes to size in pages

diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c index 788cb9faec..1f4ab2b419 100644 --- a/lib/efi_loader/efi_helper.c +++ b/lib/efi_loader/efi_helper.c @@ -223,3 +223,37 @@ bool efi_varname_is_load_option(u16 *var_name16, int *index)

return false; }

+/**

• • efi_next_variable_name() - get next variable name
• • This function is a wrapper of efi_get_next_variable_name_int().
• • If efi_get_next_variable_name_int() returns EFI_BUFFER_TOO_SMALL,
• • @size and @buf are updated by new buffer size and realloced buffer.
• • @size: pointer to the buffer size
• • @buf: pointer to the buffer
• • @guid: pointer to the guid
• • Return: status code
• */

+efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf, efi_guid_t *guid) +{

• u16 *p;
• efi_status_t ret;
• efi_uintn_t buf_size = *size;
• ret = efi_get_next_variable_name_int(&buf_size, *buf, guid);
• if (ret == EFI_NOT_FOUND)

• return ret;
  

• if (ret == EFI_BUFFER_TOO_SMALL) {

• p = realloc(*buf, buf_size);
  

• if (!p)
  

• 	return EFI_OUT_OF_RESOURCES;
  

• *buf = p;
  

• *size = buf_size;
  

• ret = efi_get_next_variable_name_int(&buf_size, *buf, guid);
  

• }
• return ret;

+}

2.17.1

Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org