* [U-Boot] [PATCH v2] cmd: env: print a message when setting UEFI variable failed
@ 2019-05-24 6:59 AKASHI Takahiro
2019-05-24 18:38 ` Heinrich Schuchardt
0 siblings, 1 reply; 4+ messages in thread
From: AKASHI Takahiro @ 2019-05-24 6:59 UTC (permalink / raw)
To: u-boot
Error message will alert a user that setting/deleting a variable failed.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@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);
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 & ~EFI_ERROR_MASK);
+ ret = CMD_RET_FAILURE;
+ }
out:
free(value);
free(var_name16);
--
2.21.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [U-Boot] [PATCH v2] cmd: env: print a message when setting UEFI variable failed
2019-05-24 6:59 [U-Boot] [PATCH v2] cmd: env: print a message when setting UEFI variable failed AKASHI Takahiro
@ 2019-05-24 18:38 ` Heinrich Schuchardt
2019-05-27 0:46 ` AKASHI Takahiro
0 siblings, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2019-05-24 18:38 UTC (permalink / raw)
To: u-boot
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@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);
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [U-Boot] [PATCH v2] cmd: env: print a message when setting UEFI variable failed
2019-05-24 18:38 ` Heinrich Schuchardt
@ 2019-05-27 0:46 ` AKASHI Takahiro
2019-05-27 10:07 ` Heinrich Schuchardt
0 siblings, 1 reply; 4+ messages in thread
From: AKASHI Takahiro @ 2019-05-27 0:46 UTC (permalink / raw)
To: u-boot
On Fri, May 24, 2019 at 08:38:41PM +0200, Heinrich Schuchardt wrote:
> 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@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.
Can you please clarify what you want me to do?
Otherwise, I cannot submit a next version.
> > 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?
Ditto.
Thanks,
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> > + ret & ~EFI_ERROR_MASK);
> > + ret = CMD_RET_FAILURE;
> > + }
> > out:
> > free(value);
> > free(var_name16);
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [U-Boot] [PATCH v2] cmd: env: print a message when setting UEFI variable failed
2019-05-27 0:46 ` AKASHI Takahiro
@ 2019-05-27 10:07 ` Heinrich Schuchardt
0 siblings, 0 replies; 4+ messages in thread
From: Heinrich Schuchardt @ 2019-05-27 10:07 UTC (permalink / raw)
To: u-boot
On 5/27/19 2:46 AM, AKASHI Takahiro wrote:
> On Fri, May 24, 2019 at 08:38:41PM +0200, Heinrich Schuchardt wrote:
>> 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@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.
>
> Can you please clarify what you want me to do?
> Otherwise, I cannot submit a next version.
I would suggest to leave the size information away.
>
>>> 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?
>
> Ditto.
I would suggest to leave the return code away.
Best regards
Heinrich
>
> Thanks,
> -Takahiro Akashi
>
>> Best regards
>>
>> Heinrich
>>
>>> + ret & ~EFI_ERROR_MASK);
>>> + ret = CMD_RET_FAILURE;
>>> + }
>>> out:
>>> free(value);
>>> free(var_name16);
>>>
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-27 10:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-24 6:59 [U-Boot] [PATCH v2] cmd: env: print a message when setting UEFI variable failed AKASHI Takahiro
2019-05-24 18:38 ` Heinrich Schuchardt
2019-05-27 0:46 ` AKASHI Takahiro
2019-05-27 10:07 ` Heinrich Schuchardt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox