Hi Heinrich,
On Fri, 23 Nov 2018 at 16:23, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/22/18 9:46 PM, Simon Glass wrote:
The functions in bootefi are very long because they mix high-level code and control with the low-level implementation. To help with this, 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 sjg@chromium.org
Changes in v15:
- Add check for return values to bootefi_test_prepare()
- Drop call to efi_save_gd() in bootefi_test_prepare()
- Fix minor checkpatch nit with bracket
Changes in v14:
- Go back to the horrible long variable names
Changes in v13:
- Rebase to efi/efi-next
Changes in v12:
- Rename image to image_prot
Changes in v11: None Changes in v9:
- Add comments to bootefi_test_prepare() about the memset()s
Changes in v7: 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
cmd/bootefi.c | 81 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 65 insertions(+), 16 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 3e37805ea13..3d98bffa542 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -453,6 +453,67 @@ 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_objp: loaded_image_infop: Pointer to a struct which will hold the
- loaded image object. This struct will be inited by this function before
- use.
- @loaded_image_infop: Pointer to a struct which will hold the loaded image
- info. This struct will be inited by this function before use.
- @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_obj **image_objp,struct efi_loaded_image **loaded_image_infop,const char *path,ulong test_func)+{
efi_status_t r;/* Construct a dummy device path */bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,(uintptr_t)test_func,(uintptr_t)test_func);if (!bootefi_device_path)return EFI_OUT_OF_RESOURCES;bootefi_image_path = efi_dp_from_file(NULL, 0, path);if (!bootefi_image_path)Please, do not leak memory.
If you mean bootefi_device_path() I am not sure how to free it as per my previous message. Also, this leak is pre-existing. I suggest you take a look when these patches land as I think you understand the naming in EFI better than I do. I cannot find any function that frees a struct efi_device_path *.
efi_free_pool(bootefi_device_path);
return EFI_OUT_OF_RESOURCES;r = efi_setup_loaded_image(bootefi_device_path, bootefi_image_path,image_objp, loaded_image_infop);if (r)return r;/* efi_save_gd() has been called in efi_init_obj_list() */This comment is only of historic interest.
OK I can remove it and send v16, but I'll hold off to see what else is happening.
/* Transfer environment variable efi_selftest as load options */set_load_options(*loaded_image_infop, "efi_selftest");return 0;+}
+/**
- bootefi_test_finish() - finish up after running an EFI test
- @image_obj: Pointer to a struct which holds the loaded image object
- @loaded_image_info: Pointer to a struct which holds the loaded image info
- */
+static void bootefi_test_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);+} +#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
static int do_bootefi_bootmgr_exec(void) { struct efi_device_path *device_path, *file_path; @@ -528,26 +589,14 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) struct efi_loaded_image_obj *image_obj;
In later patches we should get rid of any reference to struct efi_loaed_image_obj in bootefi.c. A handle is enough.
I don't understand this naming at all. Is a handle like a void *?
Regards, Simon