Hi Heinrich, On 20 June 2018 at 00:26, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > On 06/18/2018 04:08 PM, Simon Glass wrote: >> We plan to run more than one EFI test. In order to avoid duplicating code, >> create functions which handle preparing for running the test and cleaning >> up afterwards. >> >> Also shorten the awfully long variable names here. >> >> Signed-off-by: Simon Glass <s...@chromium.org> >> --- >> >> Changes in v8: None >> Changes in v7: None >> Changes in v6: None >> Changes in v5: >> - Drop call to efi_init_obj_list() which is now done in do_bootefi() >> >> Changes in v4: None >> Changes in v3: >> - Add new patch to split out test init/uninit into functions >> >> Changes in v2: None >> >> cmd/bootefi.c | 87 +++++++++++++++++++++++++++++++++++++-------------- >> 1 file changed, 63 insertions(+), 24 deletions(-) >> >> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >> index f55a40dc84..2dfd297e78 100644 >> --- a/cmd/bootefi.c >> +++ b/cmd/bootefi.c >> @@ -356,6 +356,60 @@ exit: >> return ret; >> } >> >> +#ifdef CONFIG_CMD_BOOTEFI_SELFTEST >> +/** >> + * bootefi_test_prepare() - prepare to run an EFI test >> + * >> + * This sets things up so we can call EFI functions. This involves preparing >> + * the 'gd' pointer and setting up the load ed image data structures. >> + * >> + * @image: Pointer to a struct which will hold the loaded image info >> + * @obj: Pointer to a struct which will hold the loaded image object >> + * @path: File path to the test being run (often just the test name with a >> + * backslash before it >> + * @test_func: Address of the test function that is being run >> + * @return 0 if OK, -ve on error >> + */ >> +static efi_status_t bootefi_test_prepare(struct efi_loaded_image *image, >> + struct efi_object *obj, >> + const char *path, ulong test_func) >> +{ >> + memset(image, '\0', sizeof(*image)); >> + memset(obj, '\0', sizeof(*obj)); > > It is unclear why you are adding the memsets here.
It's because they are passed in as pointers by callers, and otherwise would not inited. I will add a comment. > >> + /* Construct a dummy device path */ >> + bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, >> + (uintptr_t)test_func, >> + (uintptr_t)test_func); > > efi_dp_from_mem() may return NULL in case of an error. > >> + bootefi_image_path = efi_dp_from_file(NULL, 0, path); >> + efi_setup_loaded_image(image, obj, bootefi_device_path, >> + bootefi_image_path); > > The return code of efi_setup_loaded_image() should be checked. This > function should not return 0 if the return code is not EFI_SUCCESS. I copied the existing code, which does not check the return value anywhere. Has this function changed, or is the current code wrong? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot