| Message ID | 20190305055337.3793-5-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: rework bootefi/bootmgr | expand |
On 3/5/19 6:53 AM, AKASHI Takahiro wrote: > This is a preparatory patch. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > cmd/bootefi.c | 42 +++++++++++++++++++++--------------------- > 1 file changed, 21 insertions(+), 21 deletions(-) > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > index 3619a20e6433..1d90e7b4b575 100644 > --- a/cmd/bootefi.c > +++ b/cmd/bootefi.c > @@ -314,6 +314,27 @@ err_add_protocol: > return ret; > } > > +static int do_bootefi_bootmgr_exec(void) > +{ > + struct efi_device_path *device_path, *file_path; > + void *addr; > + efi_status_t r; > + > + addr = efi_bootmgr_load(&device_path, &file_path); > + if (!addr) > + return 1; > + > + printf("## Starting EFI application at %p ...\n", addr); > + r = do_bootefi_exec(addr, device_path, file_path); > + printf("## Application terminated, r = %lu\n", > + r & ~EFI_ERROR_MASK); > + if (r != EFI_SUCCESS) > + return 1; return CMD_RET_FAILURE ? > + > + return 0; return CMD_RET_SUCCESS ? The lines following efi_bootmgr_load() are duplicating code from do_bootefi(). The patch itself is ok. But in the patch series we should get rid of the duplication. Best regards Heinrich > +} > + > #ifdef CONFIG_CMD_BOOTEFI_SELFTEST > /** > * bootefi_test_prepare() - prepare to run an EFI test > @@ -362,27 +383,6 @@ failure: > > #endif /* CONFIG_CMD_BOOTEFI_SELFTEST */ > > -static int do_bootefi_bootmgr_exec(void) > -{ > - struct efi_device_path *device_path, *file_path; > - void *addr; > - efi_status_t r; > - > - addr = efi_bootmgr_load(&device_path, &file_path); > - if (!addr) > - return 1; > - > - printf("## Starting EFI application at %p ...\n", addr); > - r = do_bootefi_exec(addr, device_path, file_path); > - printf("## Application terminated, r = %lu\n", > - r & ~EFI_ERROR_MASK); > - > - if (r != EFI_SUCCESS) > - return 1; > - > - return 0; > -} > - > /* Interpreter command to boot an arbitrary EFI image from memory */ > static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > { >
On Thu, Mar 21, 2019 at 12:48:53PM +0100, Heinrich Schuchardt wrote: > On 3/5/19 6:53 AM, AKASHI Takahiro wrote: > > This is a preparatory patch. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > cmd/bootefi.c | 42 +++++++++++++++++++++--------------------- > > 1 file changed, 21 insertions(+), 21 deletions(-) > > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > > index 3619a20e6433..1d90e7b4b575 100644 > > --- a/cmd/bootefi.c > > +++ b/cmd/bootefi.c > > @@ -314,6 +314,27 @@ err_add_protocol: > > return ret; > > } > > > > +static int do_bootefi_bootmgr_exec(void) > > +{ > > + struct efi_device_path *device_path, *file_path; > > + void *addr; > > + efi_status_t r; > > + > > + addr = efi_bootmgr_load(&device_path, &file_path); > > + if (!addr) > > + return 1; > > + > > + printf("## Starting EFI application at %p ...\n", addr); > > + r = do_bootefi_exec(addr, device_path, file_path); > > + printf("## Application terminated, r = %lu\n", > > + r & ~EFI_ERROR_MASK); > > + if (r != EFI_SUCCESS) > > + return 1; > > return CMD_RET_FAILURE ? > > > + > > + return 0; > > return CMD_RET_SUCCESS ? > > The lines following efi_bootmgr_load() are duplicating code from > do_bootefi(). do_bootefi() -> do_boot_efi()? > The patch itself is ok. But in the patch series we should get rid of the > duplication. We only share: > > + printf("## Starting EFI application at %p ...\n", addr); > > + r = do_bootefi_exec(addr, device_path, file_path); > > + printf("## Application terminated, r = %lu\n", > > + r & ~EFI_ERROR_MASK); > > + if (r != EFI_SUCCESS) > > + return 1; Can we call it a duplication? # I don't like the print messages here anyway. -Takahiro Akashi > Best regards > > Heinrich > > > +} > > + > > #ifdef CONFIG_CMD_BOOTEFI_SELFTEST > > /** > > * bootefi_test_prepare() - prepare to run an EFI test > > @@ -362,27 +383,6 @@ failure: > > > > #endif /* CONFIG_CMD_BOOTEFI_SELFTEST */ > > > > -static int do_bootefi_bootmgr_exec(void) > > -{ > > - struct efi_device_path *device_path, *file_path; > > - void *addr; > > - efi_status_t r; > > - > > - addr = efi_bootmgr_load(&device_path, &file_path); > > - if (!addr) > > - return 1; > > - > > - printf("## Starting EFI application at %p ...\n", addr); > > - r = do_bootefi_exec(addr, device_path, file_path); > > - printf("## Application terminated, r = %lu\n", > > - r & ~EFI_ERROR_MASK); > > - > > - if (r != EFI_SUCCESS) > > - return 1; > > - > > - return 0; > > -} > > - > > /* Interpreter command to boot an arbitrary EFI image from memory */ > > static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > > { > > >
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 3619a20e6433..1d90e7b4b575 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -314,6 +314,27 @@ err_add_protocol: return ret; } +static int do_bootefi_bootmgr_exec(void) +{ + struct efi_device_path *device_path, *file_path; + void *addr; + efi_status_t r; + + addr = efi_bootmgr_load(&device_path, &file_path); + if (!addr) + return 1; + + printf("## Starting EFI application at %p ...\n", addr); + r = do_bootefi_exec(addr, device_path, file_path); + printf("## Application terminated, r = %lu\n", + r & ~EFI_ERROR_MASK); + + if (r != EFI_SUCCESS) + return 1; + + return 0; +} + #ifdef CONFIG_CMD_BOOTEFI_SELFTEST /** * bootefi_test_prepare() - prepare to run an EFI test @@ -362,27 +383,6 @@ failure: #endif /* CONFIG_CMD_BOOTEFI_SELFTEST */ -static int do_bootefi_bootmgr_exec(void) -{ - struct efi_device_path *device_path, *file_path; - void *addr; - efi_status_t r; - - addr = efi_bootmgr_load(&device_path, &file_path); - if (!addr) - return 1; - - printf("## Starting EFI application at %p ...\n", addr); - r = do_bootefi_exec(addr, device_path, file_path); - printf("## Application terminated, r = %lu\n", - r & ~EFI_ERROR_MASK); - - if (r != EFI_SUCCESS) - return 1; - - return 0; -} - /* Interpreter command to boot an arbitrary EFI image from memory */ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) {
This is a preparatory patch. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- cmd/bootefi.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-)