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.aka...@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);
> >
> 
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to