On 12/8/22 05:40, Masahisa Kojima wrote:
The current code calls efi_set_variable_int() to delete the
invalid boot option between calls to efi_get_next_variable_name_int(),
it may produce unpredictable results.
This commit moves removal of the invalid boot option outside
of the efi_get_next_variable_name_int() calls.
EFI_NOT_FOUND returned from efi_get_next_variable_name_int()
indicates we retrieve all EFI variables, it should be treated
Thanks for the correction.
%s/retrieve/retrieved/
as EFI_SUCEESS.
To address the checkpatch warning of too many leading tabs,
combine two if statement into one.
Signed-off-by: Masahisa Kojima <[email protected]>
---
cmd/eficonfig.c | 59 +++++++++++++++++++++++++++++++++++++------------
1 file changed, 45 insertions(+), 14 deletions(-)
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index cd7a51cc7e..e967d8da4d 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -2310,13 +2310,14 @@ out:
efi_status_t eficonfig_delete_invalid_boot_option(struct
eficonfig_media_boot_option *opt,
efi_status_t count)
{
- u32 i;
efi_uintn_t size;
void *load_option;
+ u32 i, list_size = 0;
struct efi_load_option lo;
u16 *var_name16 = NULL;
u16 varname[] = u"Boot####";
efi_status_t ret = EFI_SUCCESS;
+ u16 *delete_index_list = NULL, *p;
efi_uintn_t buf_size;
buf_size = 128;
@@ -2331,8 +2332,14 @@ efi_status_t eficonfig_delete_invalid_boot_option(struct
eficonfig_media_boot_op
efi_uintn_t tmp;
ret = efi_get_variable_name(&buf_size, &var_name16, &guid);
- if (ret == EFI_NOT_FOUND)
+ if (ret == EFI_NOT_FOUND) {
+ /*
+ * EFI_NOT_FOUND indicates we retrieve all EFI
variables,
%s/retrieve/retrieved/
+ * should be treated as success.
This should be ...
+ */
+ ret = EFI_SUCCESS;
break;
+ }
if (ret != EFI_SUCCESS)
goto out;
@@ -2349,31 +2356,55 @@ efi_status_t
eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_op
if (ret != EFI_SUCCESS)
goto next;
- if (size >= sizeof(efi_guid_bootmenu_auto_generated)) {
- if (guidcmp(lo.optional_data,
&efi_guid_bootmenu_auto_generated) == 0) {
- for (i = 0; i < count; i++) {
- if (opt[i].size == tmp &&
- memcmp(opt[i].lo, load_option, tmp)
== 0) {
- opt[i].exist = true;
- break;
- }
+ if (size >= sizeof(efi_guid_bootmenu_auto_generated) &&
+ guidcmp(lo.optional_data,
&efi_guid_bootmenu_auto_generated) == 0) {
According to the U-Boot programming style we prefer '!guidcmp()' over
'guidcmp() == 0'.
+ for (i = 0; i < count; i++) {
+ if (opt[i].size == tmp &&
+ memcmp(opt[i].lo, load_option, tmp) == 0) {
+ opt[i].exist = true;
+ break;
}
+ }
- if (i == count) {
- ret = delete_boot_option(i);
- if (ret != EFI_SUCCESS) {
- free(load_option);
+ /*
+ * The entire list of variables must be retrieved by
+ * efi_get_next_variable_name_int() before deleting the
invalid
+ * boot option, just save the index here.
+ */
+ if (i == count) {
+ if (!delete_index_list) {
+ delete_index_list = malloc(sizeof(u32));
The malloc branch is superfluous. Realloc called with NULL does a malloc.
Best regards
Heinrich
+ if (!delete_index_list) {
+ ret = EFI_OUT_OF_RESOURCES;
goto out;
}
+ } else {
+ p = realloc(delete_index_list,
sizeof(u32) *
+ (list_size + 1));
+ if (!p) {
+ ret = EFI_OUT_OF_RESOURCES;
+ goto out;
+ }
+ delete_index_list = p;
}
+
+ delete_index_list[list_size++] = index;
}
}
next:
free(load_option);
}
+ /* delete all invalid boot options */
+ for (i = 0; i < list_size; i++) {
+ ret = delete_boot_option(delete_index_list[i]);
+ if (ret != EFI_SUCCESS)
+ goto out;
+ }
+
out:
free(var_name16);
+ free(delete_index_list);
return ret;
}
