On 10/15/2018 04:17 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 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 | 90 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 25 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 82d755ceb31..88b8b0172db 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -454,6 +454,64 @@ 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.
- This struct will be inited by this function before use.
- @obj: Pointer to a struct which will hold the loaded image object
- 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 **imagep,
struct efi_loaded_image_obj **objp,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);- bootefi_image_path = efi_dp_from_file(NULL, 0, path);
- r = efi_setup_loaded_image(bootefi_device_path, bootefi_image_path,
objp, imagep);- if (r)
return r;- /*
* gd lives in a fixed register which may get clobbered while we execute* the payload. So save it here and restore it on every callback entry*/- efi_save_gd();
- /* Transfer environment variable efi_selftest as load options */
- set_load_options(*imagep, "efi_selftest");
- return 0;
+}
+/**
- bootefi_test_finish() - finish up after running an EFI test
- @image: Pointer to a struct which holds the loaded image info
- @obj: Pointer to a struct which holds the loaded image object
- */
+static void bootefi_test_finish(struct efi_loaded_image *image,
struct efi_loaded_image_obj *obj)+{
- efi_restore_gd();
- free(image->load_options);
- efi_delete_handle(&obj->parent);
+} +#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
static int do_bootefi_bootmgr_exec(void) { struct efi_device_path *device_path, *file_path; @@ -532,34 +590,16 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) #endif #ifdef CONFIG_CMD_BOOTEFI_SELFTEST if (!strcmp(argv[1], "selftest")) {
Why not move the whole body of the if clause to a single separate function and do the same for 'hello'.
struct efi_loaded_image_obj *image_handle;struct efi_loaded_image *loaded_image_info;/* Construct a dummy device path. */bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,(uintptr_t)&efi_selftest,(uintptr_t)&efi_selftest);bootefi_image_path = efi_dp_from_file(NULL, 0, "\\selftest");r = efi_setup_loaded_image(bootefi_device_path,bootefi_image_path, &image_handle,&loaded_image_info);if (r != EFI_SUCCESS)
struct efi_loaded_image_obj *obj;
obj what? Use a speaking name: img_handle.
struct efi_loaded_image *image;
This name obscures the variable content. The variable holds the EFI_LOADED_IMAGE_PROTOCOL. So you could call it img_prot.
Regards
Heinrich
if (bootefi_test_prepare(&image, &obj, "\\selftest",(uintptr_t)&efi_selftest)) return CMD_RET_FAILURE;
/** gd lives in a fixed register which may get clobbered while we* execute the payload. So save it here and restore it on every* callback entry*/efi_save_gd();/* Transfer environment variable efi_selftest as load options */set_load_options(loaded_image_info, "efi_selftest");r = efi_selftest(image_handle, &systab);efi_restore_gd();free(loaded_image_info->load_options);efi_delete_handle(&image_handle->parent);
r = efi_selftest(obj, &systab);bootefi_test_finish(image, obj);#endif