[U-Boot] [PATCH v11 2/6] efi_loader: Drop setup_ok

[Heinrich Schuchardt]

On 10/15/2018 04:17 PM, Simon Glass wrote:
> This value is stored in data which appears to be read-only with sandbox on
> my Ubuntu 18.04 machine. In any case it is not good practice to store
> run-time data in a build-time linker list.
Yes this should be changed. Otherwise a reset of the board will not put
us back into the initial status.

> 
> The value does not seem to be that useful, since tests that fail to setup
> are likely to fail to run also. Let's drop it for now.

With your change we will run execute() even if setup() fails. This
contradicts the commit message. Please, find a solution that avoids
calling execute() after a failed setup().

Best regards

Heinrich


> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> Changes in v11:
> - Add a new patch to drop setup_ok
> 
> Changes in v9: None
> Changes in v7: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> 
>  include/efi_selftest.h          |  2 --
>  lib/efi_selftest/efi_selftest.c | 14 +++++++-------
>  2 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/include/efi_selftest.h b/include/efi_selftest.h
> index 56beac305ec..49d3d6d0b47 100644
> --- a/include/efi_selftest.h
> +++ b/include/efi_selftest.h
> @@ -129,7 +129,6 @@ u16 efi_st_get_key(void);
>   * @setup:	set up the unit test
>   * @teardown:	tear down the unit test
>   * @execute:	execute the unit test
> - * @setup_ok:	setup was successful (set at runtime)
>   * @on_request:	test is only executed on request
>   */
>  struct efi_unit_test {
> @@ -139,7 +138,6 @@ struct efi_unit_test {
>  		     const struct efi_system_table *systable);
>  	int (*execute)(void);
>  	int (*teardown)(void);
> -	int setup_ok;
>  	bool on_request;
>  };
>  
> diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c
> index dd338db687e..dfd11be2302 100644
> --- a/lib/efi_selftest/efi_selftest.c
> +++ b/lib/efi_selftest/efi_selftest.c
> @@ -74,20 +74,20 @@ void efi_st_exit_boot_services(void)
>   */
>  static int setup(struct efi_unit_test *test, unsigned int *failures)
>  {
> -	if (!test->setup) {
> -		test->setup_ok = EFI_ST_SUCCESS;
> +	int ret;
> +
> +	if (!test->setup)
>  		return EFI_ST_SUCCESS;
> -	}
>  	efi_st_printc(EFI_LIGHTBLUE, "\nSetting up '%s'\n", test->name);
> -	test->setup_ok = test->setup(handle, systable);
> -	if (test->setup_ok != EFI_ST_SUCCESS) {
> +	ret = test->setup(handle, systable);
> +	if (ret) {
>  		efi_st_error("Setting up '%s' failed\n", test->name);
>  		++*failures;
>  	} else {
>  		efi_st_printc(EFI_LIGHTGREEN,
>  			      "Setting up '%s' succeeded\n", test->name);
>  	}
> -	return test->setup_ok;
> +	return ret;
>  }
>  
>  /*
> @@ -197,7 +197,7 @@ void efi_st_do_tests(const u16 *testname, unsigned int phase,
>  			continue;
>  		if (steps & EFI_ST_SETUP)
>  			setup(test, failures);
> -		if (steps & EFI_ST_EXECUTE && test->setup_ok == EFI_ST_SUCCESS)
> +		if (steps & EFI_ST_EXECUTE)
>  			execute(test, failures);
>  		if (steps & EFI_ST_TEARDOWN)
>  			teardown(test, failures);
>