Hi Alex, On Sat, 24 Nov 2018 at 14:26, Alexander Graf <ag...@suse.de> wrote: > > > > On 22.11.18 21:46, Simon Glass wrote: > > This function can be used from do_bootefi_exec() so that we use mostly the > > same code for a normal EFI application and an EFI test. > > > > Rename the function and use it in both places. > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > --- > > > > Changes in v15: > > - Add a comment about a leaked device path > > > > Changes in v14: > > - Go back to the horrible long variable names > > - Hopefully correct error paths in do_bootefi_exec() > > > > Changes in v13: > > - Drop 'efi_loader: Drop setup_ok' as we have an existing patch for that > > - Drop patches previously applied > > > > Changes in v12: None > > Changes in v11: > > - Drop patches previously applied > > > > Changes in v9: None > > Changes in v7: > > - Drop patch "efi: Init the 'rows' and 'cols' variables" > > - Drop patches previous applied > > > > Changes in v5: > > - Rebase to master > > > > Changes in v4: > > - Rebase to master > > > > Changes in v3: > > - Add new patch to rename bootefi_test_finish() to bootefi_run_finish() > > > > cmd/bootefi.c | 46 ++++++++++++++++++++++++---------------------- > > 1 file changed, 24 insertions(+), 22 deletions(-) > > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > > index 0ca84ff7168..5831c991a8e 100644 > > --- a/cmd/bootefi.c > > +++ b/cmd/bootefi.c > > @@ -346,6 +346,20 @@ static efi_status_t bootefi_run_prepare(const char > > *load_options_path, > > return 0; > > } > > > > +/** > > + * bootefi_run_finish() - finish up after running an EFI test > > + * > > + * @loaded_image_info: Pointer to a struct which holds the loaded image > > info > > + * @image_objj: Pointer to a struct which holds the loaded image object > > + */ > > +static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj, > > + struct efi_loaded_image *loaded_image_info) > > +{ > > + efi_restore_gd(); > > + free(loaded_image_info->load_options); > > + efi_delete_handle(&image_obj->header); > > +} > > + > > /** > > * do_bootefi_exec() - execute EFI binary > > * > > @@ -386,11 +400,11 @@ static efi_status_t do_bootefi_exec(void *efi, > > */ > > ret = efi_create_handle(&mem_handle); > > if (ret != EFI_SUCCESS) > > - goto exit; > > + return ret; /* TODO: leaks device_path */ > > ret = efi_add_protocol(mem_handle, &efi_guid_device_path, > > device_path); > > if (ret != EFI_SUCCESS) > > - goto exit; > > + goto err_add_protocol; > > } else { > > assert(device_path && image_path); > > } > > @@ -398,13 +412,13 @@ static efi_status_t do_bootefi_exec(void *efi, > > ret = bootefi_run_prepare("bootargs", device_path, image_path, > > &image_obj, &loaded_image_info); > > if (ret) > > - return ret; > > + goto err_prepare; > > > > /* Load the EFI payload */ > > entry = efi_load_pe(image_obj, efi, loaded_image_info); > > if (!entry) { > > ret = EFI_LOAD_ERROR; > > - goto exit; > > + goto err_prepare; > > } > > > > if (memdp) { > > @@ -424,7 +438,7 @@ static efi_status_t do_bootefi_exec(void *efi, > > > > if (setjmp(&image_obj->exit_jmp)) { > > ret = image_obj->exit_status; > > - goto exit; > > + goto err_prepare; > > } > > > > #ifdef CONFIG_ARM64 > > @@ -462,10 +476,11 @@ static efi_status_t do_bootefi_exec(void *efi, > > > > ret = efi_do_enter(&image_obj->header, &systab, entry); > > > > -exit: > > +err_prepare: > > /* image has returned, loaded-image obj goes *poof*: */ > > - if (image_obj) > > - efi_delete_handle(&image_obj->header); > > + bootefi_run_finish(image_obj, loaded_image_info); > > So here we now free loaded_image_info->load_options which we didn't do > before. That means the patch does change behavior. > > I think the change is correct though. For the sake of bisectability, I'd > prefer if you could add a tiny patch before this patch that just adds > the free(loaded_image_info->load_options) in this spot. We can then have > this refactoring really be neutral as to behavior.
I agree, it worries me that we are fixing up complex code in a refactor. Can you just use v14? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot