[PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command

[Heinrich Schuchardt]

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 at 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 at 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) {
>>>
>>