[U-Boot] [PATCH v2] cmd: env: print a message when setting UEFI variable failed

[Heinrich Schuchardt]

On 5/24/19 8:59 AM, AKASHI Takahiro wrote:
> Error message will alert a user that setting/deleting a variable failed.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
> Changes in v2 (May 24, 2019)
> * change a message to "Out of memory"
> * add more error messages
> ---
>  cmd/nvedit_efi.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/nvedit_efi.c b/cmd/nvedit_efi.c
> index 2805e8182b41..88d6ca1cd7f2 100644
> --- a/cmd/nvedit_efi.c
> +++ b/cmd/nvedit_efi.c
> @@ -373,6 +373,8 @@ int do_env_set_efi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>
>  		for ( ; argc > 0; argc--, argv++)
>  			if (append_value(&value, &size, argv[0]) < 0) {
> +				printf("## Failed to process an argument, %s\n",
> +				       argv[0]);
>  				ret = CMD_RET_FAILURE;
>  				goto out;
>  			}
> @@ -381,6 +383,7 @@ int do_env_set_efi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  	len = utf8_utf16_strnlen(var_name, strlen(var_name));
>  	var_name16 = malloc((len + 1) * 2);
>  	if (!var_name16) {
> +		printf("## Out of memory (%ld bytes)\n", (len + 1) * 2);

cmd/nvedit_efi.c: In function ‘do_env_set_efi’:
cmd/nvedit_efi.c:386:31: warning: format ‘%ld’ expects argument of type
‘long int’, but argument 2 has type ‘size_t’ {aka ‘unsigned int’}
[-Wformat=]
   printf("## Out of memory (%ld bytes)\n", (len + 1) * 2);
                             ~~^            ~~~~~~~~~~~~~
                             %d

If you want to print the number of bytes use %zu. But why should the
user care if it is 10 bytes or 100 bytes that could not be assigned? If
he is out of memory here anyway the system is messed up.

>  		ret = CMD_RET_FAILURE;
>  		goto out;
>  	}
> @@ -392,7 +395,13 @@ int do_env_set_efi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  					EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  					EFI_VARIABLE_RUNTIME_ACCESS,
>  					size, value));
> -	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> +	if (ret == EFI_SUCCESS) {
> +		ret = CMD_RET_SUCCESS;
> +	} else {
> +		printf("## Failed to set EFI variable (%ld)\n",

ret has type size_t so this has to %zu.

Think of a non-developer user who bought a device with pre-installedö
U-Boot. What would he do with the error code?

Best regards

Heinrich

> +		       ret & ~EFI_ERROR_MASK);
> +		ret = CMD_RET_FAILURE;
> +	}
>  out:
>  	free(value);
>  	free(var_name16);
>