From mboxrd@z Thu Jan  1 00:00:00 1970
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
Date: Fri, 22 Mar 2019 11:16:35 +0900
Subject: [U-Boot] [RFC 4/8] cmd: bootefi: move do_bootefi_bootmgr_exec()
	forward
In-Reply-To: <92c0ca2c-cd99-9790-b6d2-42c3095b3dd6@gmx.de>
References: <20190305055337.3793-1-takahiro.akashi@linaro.org>
 <20190305055337.3793-5-takahiro.akashi@linaro.org>
 <92c0ca2c-cd99-9790-b6d2-42c3095b3dd6@gmx.de>
Message-ID: <20190322021634.GN9937@linaro.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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[])
> >  {
> >
>