* [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command
@ 2020-02-28 0:05 AKASHI Takahiro
2020-02-28 18:05 ` Heinrich Schuchardt
0 siblings, 1 reply; 4+ messages in thread
From: AKASHI Takahiro @ 2020-02-28 0:05 UTC (permalink / raw)
To: u-boot
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);
ret = EFI_CALL(RT->set_variable(var_name16, &guid, 0, 0, NULL));
if (ret) {
--
2.24.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command
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
0 siblings, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2020-02-28 18:05 UTC (permalink / raw)
To: u-boot
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:
??????? 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);
try_load_entry() has another implementation.
Best regards
Heinrich
>
> ret = EFI_CALL(RT->set_variable(var_name16, &guid, 0, 0, NULL));
> if (ret) {
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command
2020-02-28 18:05 ` Heinrich Schuchardt
@ 2020-03-02 0:05 ` AKASHI Takahiro
2020-03-02 19:22 ` Heinrich Schuchardt
0 siblings, 1 reply; 4+ messages in thread
From: AKASHI Takahiro @ 2020-03-02 0:05 UTC (permalink / raw)
To: u-boot
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) {
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command
2020-03-02 0:05 ` AKASHI Takahiro
@ 2020-03-02 19:22 ` Heinrich Schuchardt
0 siblings, 0 replies; 4+ messages in thread
From: Heinrich Schuchardt @ 2020-03-02 19:22 UTC (permalink / raw)
To: u-boot
On 3/2/20 1:05 AM, AKASHI Takahiro wrote:
> 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.
Factoring out a separate function does not reduce the code size.
Hence
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
>> ??????? 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) {
>>>
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-02 19:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2020-03-02 19:22 ` Heinrich Schuchardt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.