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 masahisa.kojima@linaro.org
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;}@@ -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; }