On Tue, 6 Jun 2023 at 22:23, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > Hi Raymond > > On Tue, 6 Jun 2023 at 19:37, Raymond Mao <raymond....@linaro.org> wrote: > > > > Correct the return code for out-of-memory and no boot option found > > > > Signed-off-by: Raymond Mao <raymond....@linaro.org> > > Reviewed-by: Ilias Apalodimas <ilias.apalodi...@linaro.org> > > --- > > Changes in v7 > > - new patch file created > > > > cmd/bootmenu.c | 2 +- > > cmd/eficonfig.c | 2 +- > > lib/efi_loader/efi_bootmgr.c | 8 ++++++-- > > 3 files changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c > > index 01daddca7b..987b16889f 100644 > > --- a/cmd/bootmenu.c > > +++ b/cmd/bootmenu.c > > @@ -352,7 +352,7 @@ static struct bootmenu_data *bootmenu_create(int delay) > > * a architecture-specific default image name such as > > BOOTAA64.EFI. > > */ > > efi_ret = efi_bootmgr_update_media_device_boot_option(); > > - if (efi_ret != EFI_SUCCESS && efi_ret != EFI_NOT_FOUND) > > + if (efi_ret != EFI_SUCCESS) > > goto cleanup; > > > > ret = prepare_uefi_bootorder_entry(menu, &iter, &i); > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c > > index 82a80306f4..e6e8a0a488 100644 > > --- a/cmd/eficonfig.c > > +++ b/cmd/eficonfig.c > > @@ -2314,7 +2314,7 @@ static int do_eficonfig(struct cmd_tbl *cmdtp, int > > flag, int argc, char *const a > > return CMD_RET_FAILURE; > > > > ret = efi_bootmgr_update_media_device_boot_option(); > > - if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND) > > + if (ret != EFI_SUCCESS) > > return ret; > > > > while (1) { > > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c > > index 97d5b8dc2b..28e800499c 100644 > > --- a/lib/efi_loader/efi_bootmgr.c > > +++ b/lib/efi_loader/efi_bootmgr.c > > @@ -663,11 +663,13 @@ efi_status_t > > efi_bootmgr_update_media_device_boot_option(void) > > NULL, &count, > > (efi_handle_t > > **)&volume_handles); > > if (ret != EFI_SUCCESS) > > - return ret; > > + goto out; > > I missed that in my original review, but I think this change is wrong. > If you follow the goto out tag now instead of returning directly > there's a chance efi_locate_handle_buffer_int() will return > EFI_NOT_FOUND. The goto out tag will convert that to EFI_SUCCESS > although it's an actual error.
OTOH we were ignoring EFI_NOT_FOUND already outside this function. So the overall functionality isn't affected Thanks /Ilias > > Thanks > /Ilias > > > > opt = calloc(count, sizeof(struct eficonfig_media_boot_option)); > > - if (!opt) > > + if (!opt) { > > + ret = EFI_OUT_OF_RESOURCES; > > goto out; > > + } > > > > /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL > > */ > > ret = efi_bootmgr_enumerate_boot_option(opt, volume_handles, count); > > @@ -720,5 +722,7 @@ out: > > free(opt); > > efi_free_pool(volume_handles); > > > > + if (ret == EFI_NOT_FOUND) > > + return EFI_SUCCESS; > > return ret; > > } > > -- > > 2.25.1 > >