On Fri, Feb 28, 2020 at 07:05:39PM +0100, Heinrich Schuchardt wrote: > On 2/28/20 1:05 AM, AKASHI Takahiro wrote: > > There is a wrong usage of utf8_utf16_strncpy() in "boot rm" command, and > > then it will end up with a failure of this command due to a wrong > > value of an interim variable ("var_name16"). > > > > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org> > > --- > > cmd/efidebug.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/cmd/efidebug.c b/cmd/efidebug.c > > index 576e95b395dc..3a50dafbbca6 100644 > > --- a/cmd/efidebug.c > > +++ b/cmd/efidebug.c > > @@ -641,7 +641,7 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, > > int id, i; > > char *endp; > > char var_name[9]; > > - u16 var_name16[9]; > > + u16 var_name16[9], *p; > > efi_status_t ret; > > > > if (argc == 1) > > @@ -654,7 +654,8 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, > > return CMD_RET_FAILURE; > > > > sprintf(var_name, "Boot%04X", id); > > - utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9); > > + p = var_name16; > > + utf8_utf16_strncpy(&p, var_name, 9); > > This is duplicating code in do_efi_boot_add(). Please, consider putting > the following codeblock into separate function:
No. > id = (int)simple_strtoul(argv[1], &endp, 16); > if (*endp != '\0' || id > 0xffff) > return CMD_RET_USAGE; > > sprintf(var_name, "Boot%04X", id); > p = var_name16; > utf8_utf16_strncpy(&p, var_name, 9); Frankly, I don't like this function's prototype because it is different from "normal" corresponding C libraries. Moreover, as far as I notice, there is no use case where the first parameter which is modified after calling is used in any code. Thanks, -Takahiro Akashi > try_load_entry() has another implementation. > > Best regards > > Heinrich > > > > > ret = EFI_CALL(RT->set_variable(var_name16, &guid, 0, 0, NULL)); > > if (ret) { > > >