Akashi-san
On Fri, 27 Oct 2023 at 04:00, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 27, 2023 at 09:25:44AM +0900, AKASHI Takahiro wrote:
On Thu, Oct 26, 2023 at 01:01:52PM +0200, Heinrich Schuchardt wrote:
On 10/26/23 07:30, AKASHI Takahiro wrote:
Unfold do_bootefi_image() into do_bootefi() for the sake of the succeeding refactor work.
I don't disagree with the patchset, but what the patch does is pretty obvious. It would help a lot to briefly describe how the unfolding process helps the refactoring.
Thanks /Ilias
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/bootefi.c | 101 ++++++++++++++++++-------------------------------- 1 file changed, 37 insertions(+), 64 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 20e5c94a33a4..1b28bf5a318d 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -425,58 +425,6 @@ static int do_efibootmgr(void) return CMD_RET_SUCCESS; }
-/**
- do_bootefi_image() - execute EFI binary
- Set up memory image for the binary to be loaded, prepare device path, and
- then call do_bootefi_exec() to execute it.
- @image_opt: string with image start address
- @size_opt: string with image size or NULL
- Return: status code
- */
-static int do_bootefi_image(const char *image_opt, const char *size_opt) -{
- void *image_buf;
- unsigned long addr, size;
- efi_status_t ret;
-#ifdef CONFIG_CMD_BOOTEFI_HELLO
- if (!strcmp(image_opt, "hello")) {
image_buf = __efi_helloworld_begin;size = __efi_helloworld_end - __efi_helloworld_begin;efi_clear_bootdev();- } else
-#endif
- {
addr = strtoul(image_opt, NULL, 16);/* Check that a numeric value was passed */if (!addr)return CMD_RET_USAGE;image_buf = map_sysmem(addr, 0);if (size_opt) {size = strtoul(size_opt, NULL, 16);if (!size)return CMD_RET_USAGE;efi_clear_bootdev();} else {if (image_buf != image_addr) {log_err("No UEFI binary known at %s\n",image_opt);return CMD_RET_FAILURE;}size = image_size;}- }
- ret = efi_run_image(image_buf, size);
- if (ret != EFI_SUCCESS)
return CMD_RET_FAILURE;- return CMD_RET_SUCCESS;
-}
- /**
- efi_run_image() - run loaded UEFI image
@@ -648,8 +596,9 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { efi_status_t ret;
- char *img_addr, *img_size, *str_copy, *pos;
- void *fdt;
char *p;
void *fdt, *image_buf;
unsigned long addr, size;
if (argc < 2) return CMD_RET_USAGE;@@ -684,18 +633,42 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc, if (!strcmp(argv[1], "selftest")) return do_efi_selftest(); #endif
- str_copy = strdup(argv[1]);
- if (!str_copy) {
log_err("Out of memory\n");return CMD_RET_FAILURE;
+#ifdef CONFIG_CMD_BOOTEFI_HELLO
I would prefer a normal if and let the linker sort it out.
Here I focused on simply unfolding("moving") the function, keeping the code as same as possible. Any how the code is in a tentative form at this point and will be reshaped in later patches, using IS_ENABLED(). Please look at the final code after applying the whole series.
I also agree with this approach.
-- Tom