From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: u-boot@lists.denx.de
Subject: [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command
Date: Mon, 2 Mar 2020 09:05:29 +0900 [thread overview]
Message-ID: <20200302000528.GC13880@linaro.org> (raw)
In-Reply-To: <022ee63f-12af-3029-dfd4-f0e286eb8c5b@gmx.de>
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.akashi@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) {
> >
>
next prev parent reply other threads:[~2020-03-02 0:05 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-28 0:05 [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command AKASHI Takahiro
2020-02-28 18:05 ` Heinrich Schuchardt
2020-03-02 0:05 ` AKASHI Takahiro [this message]
2020-03-02 19:22 ` Heinrich Schuchardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200302000528.GC13880@linaro.org \
--to=takahiro.akashi@linaro.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox