| Message ID | 20220526100938.9558-2-masahisa.kojima@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fix issues in bootmenu after adding efi entries | expand |
On 5/26/22 12:09, Masahisa Kojima wrote: > The commit a3d0aa87acbe ("bootmenu: update bootmenu_entry structure") > changes the bootmenu title type from char to u16(UTF16 string) > to support EFI based system. If EFI_LOADER is not enabled, > printf("%ls") is not supported, so bootmenu does not appear > correctly. > > This commit changes the type of menu title from u16(UTF16) to > utf-8 string and EFI strings is conveted into utf-8. > > Fixes: a3d0aa87acbe ("bootmenu: update bootmenu_entry structure") > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > Tested-by: Pali Rohar <pali@kernel.org> > --- > > (no change since v1) > > cmd/bootmenu.c | 36 +++++++++++++++++++++--------------- > 1 file changed, 21 insertions(+), 15 deletions(-) > > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c > index 8859eebea5..bf88c2127b 100644 > --- a/cmd/bootmenu.c > +++ b/cmd/bootmenu.c > @@ -43,7 +43,7 @@ enum boot_type { > struct bootmenu_entry { > unsigned short int num; /* unique number 0 .. MAX_COUNT */ > char key[3]; /* key identifier of number */ > - u16 *title; /* title of entry */ > + char *title; /* title of entry */ > char *command; /* hush command of entry */ > enum boot_type type; /* boot type of entry */ > u16 bootorder; /* order for each boot type */ > @@ -76,7 +76,7 @@ static void bootmenu_print_entry(void *data) > if (reverse) > puts(ANSI_COLOR_REVERSE); > > - printf("%ls", entry->title); > + printf("%s", entry->title); > > if (reverse) > puts(ANSI_COLOR_RESET); > @@ -170,7 +170,7 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu, > struct bootmenu_entry *iter = *current; > > while ((option = bootmenu_getoption(i))) { > - u16 *buf; > + char *buf; > > sep = strchr(option, '='); > if (!sep) { > @@ -183,13 +183,13 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu, > return -ENOMEM; > > len = sep-option; %s/sep-option/sep - option/ > - buf = calloc(1, (len + 1) * sizeof(u16)); > + buf = calloc(1, (len + 1)); > entry->title = buf; > if (!entry->title) { > free(entry); > return -ENOMEM; > } > - utf8_utf16_strncpy(&buf, option, len); > + strncpy(buf, option, len); Instead of two function calls (calloc, strncpy) simply use strdup(). > > len = strlen(sep + 1); > entry->command = malloc(len + 1); Use strdup() here too. Best regards Heinrich > @@ -227,6 +227,7 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu, > return 1; > } > > +#if (CONFIG_IS_ENABLED(CMD_BOOTEFI_BOOTMGR)) > /** > * prepare_uefi_bootorder_entry() - generate the uefi bootmenu entries > * > @@ -279,13 +280,17 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, > } > > if (lo.attributes & LOAD_OPTION_ACTIVE) { > - entry->title = u16_strdup(lo.label); > - if (!entry->title) { > + char *buf; > + > + buf = calloc(1, utf16_utf8_strlen(lo.label) + 1); > + if (!buf) { > free(load_option); > free(entry); > free(bootorder); > return -ENOMEM; > } > + entry->title = buf; > + utf16_utf8_strncpy(&buf, lo.label, u16_strlen(lo.label)); > entry->command = strdup("bootefi bootmgr"); > sprintf(entry->key, "%d", i); > entry->num = i; > @@ -315,6 +320,7 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, > > return 1; > } > +#endif > > static struct bootmenu_data *bootmenu_create(int delay) > { > @@ -341,13 +347,13 @@ static struct bootmenu_data *bootmenu_create(int delay) > if (ret < 0) > goto cleanup; > > - if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { > - if (i < MAX_COUNT - 1) { > +#if (CONFIG_IS_ENABLED(CMD_BOOTEFI_BOOTMGR)) > + if (i < MAX_COUNT - 1) { > ret = prepare_uefi_bootorder_entry(menu, &iter, &i); > if (ret < 0 && ret != -ENOENT) > goto cleanup; > - } > } > +#endif > > /* Add U-Boot console entry at the end */ > if (i <= MAX_COUNT - 1) { > @@ -357,9 +363,9 @@ static struct bootmenu_data *bootmenu_create(int delay) > > /* Add Quit entry if entering U-Boot console is disabled */ > if (IS_ENABLED(CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE)) > - entry->title = u16_strdup(u"U-Boot console"); > + entry->title = strdup("U-Boot console"); > else > - entry->title = u16_strdup(u"Quit"); > + entry->title = strdup("Quit"); > > if (!entry->title) { > free(entry); > @@ -461,7 +467,7 @@ static enum bootmenu_ret bootmenu_show(int delay) > int cmd_ret; > int init = 0; > void *choice = NULL; > - u16 *title = NULL; > + char *title = NULL; > char *command = NULL; > struct menu *menu; > struct bootmenu_entry *iter; > @@ -517,7 +523,7 @@ static enum bootmenu_ret bootmenu_show(int delay) > > if (menu_get_choice(menu, &choice) == 1) { > iter = choice; > - title = u16_strdup(iter->title); > + title = strdup(iter->title); > command = strdup(iter->command); > > /* last entry is U-Boot console or Quit */ > @@ -561,7 +567,7 @@ cleanup: > } > > if (title && command) { > - debug("Starting entry '%ls'\n", title); > + debug("Starting entry '%s'\n", title); > free(title); > if (efi_ret == EFI_SUCCESS) > cmd_ret = run_command(command, 0);
On Sat, 28 May 2022 at 17:37, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 5/26/22 12:09, Masahisa Kojima wrote: > > The commit a3d0aa87acbe ("bootmenu: update bootmenu_entry structure") > > changes the bootmenu title type from char to u16(UTF16 string) > > to support EFI based system. If EFI_LOADER is not enabled, > > printf("%ls") is not supported, so bootmenu does not appear > > correctly. > > > > This commit changes the type of menu title from u16(UTF16) to > > utf-8 string and EFI strings is conveted into utf-8. > > > > Fixes: a3d0aa87acbe ("bootmenu: update bootmenu_entry structure") > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > > Tested-by: Pali Rohar <pali@kernel.org> > > --- > > > > (no change since v1) > > > > cmd/bootmenu.c | 36 +++++++++++++++++++++--------------- > > 1 file changed, 21 insertions(+), 15 deletions(-) > > > > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c > > index 8859eebea5..bf88c2127b 100644 > > --- a/cmd/bootmenu.c > > +++ b/cmd/bootmenu.c > > @@ -43,7 +43,7 @@ enum boot_type { > > struct bootmenu_entry { > > unsigned short int num; /* unique number 0 .. MAX_COUNT */ > > char key[3]; /* key identifier of number */ > > - u16 *title; /* title of entry */ > > + char *title; /* title of entry */ > > char *command; /* hush command of entry */ > > enum boot_type type; /* boot type of entry */ > > u16 bootorder; /* order for each boot type */ > > @@ -76,7 +76,7 @@ static void bootmenu_print_entry(void *data) > > if (reverse) > > puts(ANSI_COLOR_REVERSE); > > > > - printf("%ls", entry->title); > > + printf("%s", entry->title); > > > > if (reverse) > > puts(ANSI_COLOR_RESET); > > @@ -170,7 +170,7 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu, > > struct bootmenu_entry *iter = *current; > > > > while ((option = bootmenu_getoption(i))) { > > - u16 *buf; > > + char *buf; > > > > sep = strchr(option, '='); > > if (!sep) { > > @@ -183,13 +183,13 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu, > > return -ENOMEM; > > > > len = sep-option; > > %s/sep-option/sep - option/ OK. > > > - buf = calloc(1, (len + 1) * sizeof(u16)); > > + buf = calloc(1, (len + 1)); > > entry->title = buf; > > if (!entry->title) { > > free(entry); > > return -ENOMEM; > > } > > - utf8_utf16_strncpy(&buf, option, len); > > + strncpy(buf, option, len); > > Instead of two function calls (calloc, strncpy) simply use strdup(). bootmenu_x format is "[title]=[commands]", title is not the NUL-terminated string, so we can not use strdup here. > > > > > len = strlen(sep + 1); > > entry->command = malloc(len + 1); > > Use strdup() here too. OK, command is NUL-terminated. Thank you for your comment. Regards, Masahisa Kojima > > Best regards > > Heinrich > > > @@ -227,6 +227,7 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu, > > return 1; > > } > > > > +#if (CONFIG_IS_ENABLED(CMD_BOOTEFI_BOOTMGR)) > > /** > > * prepare_uefi_bootorder_entry() - generate the uefi bootmenu entries > > * > > @@ -279,13 +280,17 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, > > } > > > > if (lo.attributes & LOAD_OPTION_ACTIVE) { > > - entry->title = u16_strdup(lo.label); > > - if (!entry->title) { > > + char *buf; > > + > > + buf = calloc(1, utf16_utf8_strlen(lo.label) + 1); > > + if (!buf) { > > free(load_option); > > free(entry); > > free(bootorder); > > return -ENOMEM; > > } > > + entry->title = buf; > > + utf16_utf8_strncpy(&buf, lo.label, u16_strlen(lo.label)); > > entry->command = strdup("bootefi bootmgr"); > > sprintf(entry->key, "%d", i); > > entry->num = i; > > @@ -315,6 +320,7 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, > > > > return 1; > > } > > +#endif > > > > static struct bootmenu_data *bootmenu_create(int delay) > > { > > @@ -341,13 +347,13 @@ static struct bootmenu_data *bootmenu_create(int delay) > > if (ret < 0) > > goto cleanup; > > > > - if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { > > - if (i < MAX_COUNT - 1) { > > +#if (CONFIG_IS_ENABLED(CMD_BOOTEFI_BOOTMGR)) > > + if (i < MAX_COUNT - 1) { > > ret = prepare_uefi_bootorder_entry(menu, &iter, &i); > > if (ret < 0 && ret != -ENOENT) > > goto cleanup; > > - } > > } > > +#endif > > > > /* Add U-Boot console entry at the end */ > > if (i <= MAX_COUNT - 1) { > > @@ -357,9 +363,9 @@ static struct bootmenu_data *bootmenu_create(int delay) > > > > /* Add Quit entry if entering U-Boot console is disabled */ > > if (IS_ENABLED(CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE)) > > - entry->title = u16_strdup(u"U-Boot console"); > > + entry->title = strdup("U-Boot console"); > > else > > - entry->title = u16_strdup(u"Quit"); > > + entry->title = strdup("Quit"); > > > > if (!entry->title) { > > free(entry); > > @@ -461,7 +467,7 @@ static enum bootmenu_ret bootmenu_show(int delay) > > int cmd_ret; > > int init = 0; > > void *choice = NULL; > > - u16 *title = NULL; > > + char *title = NULL; > > char *command = NULL; > > struct menu *menu; > > struct bootmenu_entry *iter; > > @@ -517,7 +523,7 @@ static enum bootmenu_ret bootmenu_show(int delay) > > > > if (menu_get_choice(menu, &choice) == 1) { > > iter = choice; > > - title = u16_strdup(iter->title); > > + title = strdup(iter->title); > > command = strdup(iter->command); > > > > /* last entry is U-Boot console or Quit */ > > @@ -561,7 +567,7 @@ cleanup: > > } > > > > if (title && command) { > > - debug("Starting entry '%ls'\n", title); > > + debug("Starting entry '%s'\n", title); > > free(title); > > if (efi_ret == EFI_SUCCESS) > > cmd_ret = run_command(command, 0); >
On Sunday 29 May 2022 10:37:23 Masahisa Kojima wrote: > On Sat, 28 May 2022 at 17:37, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 5/26/22 12:09, Masahisa Kojima wrote: > > > @@ -183,13 +183,13 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu, > > > return -ENOMEM; > > > > > > len = sep-option; > > > > %s/sep-option/sep - option/ > > OK. > > > > > > - buf = calloc(1, (len + 1) * sizeof(u16)); > > > + buf = calloc(1, (len + 1)); > > > entry->title = buf; > > > if (!entry->title) { > > > free(entry); > > > return -ENOMEM; > > > } > > > - utf8_utf16_strncpy(&buf, option, len); > > > + strncpy(buf, option, len); > > > > Instead of two function calls (calloc, strncpy) simply use strdup(). > > bootmenu_x format is "[title]=[commands]", title is not the > NUL-terminated string, so we can not use strdup here. Usage of C's strncpy() function should be in most cases avoided as when target buffer does not have enough space this function does not fill nul-term byte. (Anyway, I do not know if U-Boot implements strncpy() according to C standard with this trap or not) But if you already have length of string then you should use memcpy(). > > > > > > > > len = strlen(sep + 1); > > > entry->command = malloc(len + 1); > > > > Use strdup() here too. > > OK, command is NUL-terminated. > > Thank you for your comment. > > Regards, > Masahisa Kojima
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index 8859eebea5..bf88c2127b 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -43,7 +43,7 @@ enum boot_type { struct bootmenu_entry { unsigned short int num; /* unique number 0 .. MAX_COUNT */ char key[3]; /* key identifier of number */ - u16 *title; /* title of entry */ + char *title; /* title of entry */ char *command; /* hush command of entry */ enum boot_type type; /* boot type of entry */ u16 bootorder; /* order for each boot type */ @@ -76,7 +76,7 @@ static void bootmenu_print_entry(void *data) if (reverse) puts(ANSI_COLOR_REVERSE); - printf("%ls", entry->title); + printf("%s", entry->title); if (reverse) puts(ANSI_COLOR_RESET); @@ -170,7 +170,7 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu, struct bootmenu_entry *iter = *current; while ((option = bootmenu_getoption(i))) { - u16 *buf; + char *buf; sep = strchr(option, '='); if (!sep) { @@ -183,13 +183,13 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu, return -ENOMEM; len = sep-option; - buf = calloc(1, (len + 1) * sizeof(u16)); + buf = calloc(1, (len + 1)); entry->title = buf; if (!entry->title) { free(entry); return -ENOMEM; } - utf8_utf16_strncpy(&buf, option, len); + strncpy(buf, option, len); len = strlen(sep + 1); entry->command = malloc(len + 1); @@ -227,6 +227,7 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu, return 1; } +#if (CONFIG_IS_ENABLED(CMD_BOOTEFI_BOOTMGR)) /** * prepare_uefi_bootorder_entry() - generate the uefi bootmenu entries * @@ -279,13 +280,17 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, } if (lo.attributes & LOAD_OPTION_ACTIVE) { - entry->title = u16_strdup(lo.label); - if (!entry->title) { + char *buf; + + buf = calloc(1, utf16_utf8_strlen(lo.label) + 1); + if (!buf) { free(load_option); free(entry); free(bootorder); return -ENOMEM; } + entry->title = buf; + utf16_utf8_strncpy(&buf, lo.label, u16_strlen(lo.label)); entry->command = strdup("bootefi bootmgr"); sprintf(entry->key, "%d", i); entry->num = i; @@ -315,6 +320,7 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, return 1; } +#endif static struct bootmenu_data *bootmenu_create(int delay) { @@ -341,13 +347,13 @@ static struct bootmenu_data *bootmenu_create(int delay) if (ret < 0) goto cleanup; - if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { - if (i < MAX_COUNT - 1) { +#if (CONFIG_IS_ENABLED(CMD_BOOTEFI_BOOTMGR)) + if (i < MAX_COUNT - 1) { ret = prepare_uefi_bootorder_entry(menu, &iter, &i); if (ret < 0 && ret != -ENOENT) goto cleanup; - } } +#endif /* Add U-Boot console entry at the end */ if (i <= MAX_COUNT - 1) { @@ -357,9 +363,9 @@ static struct bootmenu_data *bootmenu_create(int delay) /* Add Quit entry if entering U-Boot console is disabled */ if (IS_ENABLED(CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE)) - entry->title = u16_strdup(u"U-Boot console"); + entry->title = strdup("U-Boot console"); else - entry->title = u16_strdup(u"Quit"); + entry->title = strdup("Quit"); if (!entry->title) { free(entry); @@ -461,7 +467,7 @@ static enum bootmenu_ret bootmenu_show(int delay) int cmd_ret; int init = 0; void *choice = NULL; - u16 *title = NULL; + char *title = NULL; char *command = NULL; struct menu *menu; struct bootmenu_entry *iter; @@ -517,7 +523,7 @@ static enum bootmenu_ret bootmenu_show(int delay) if (menu_get_choice(menu, &choice) == 1) { iter = choice; - title = u16_strdup(iter->title); + title = strdup(iter->title); command = strdup(iter->command); /* last entry is U-Boot console or Quit */ @@ -561,7 +567,7 @@ cleanup: } if (title && command) { - debug("Starting entry '%ls'\n", title); + debug("Starting entry '%s'\n", title); free(title); if (efi_ret == EFI_SUCCESS) cmd_ret = run_command(command, 0);
The commit a3d0aa87acbe ("bootmenu: update bootmenu_entry structure") changes the bootmenu title type from char to u16(UTF16 string) to support EFI based system. If EFI_LOADER is not enabled, printf("%ls") is not supported, so bootmenu does not appear correctly. This commit changes the type of menu title from u16(UTF16) to utf-8 string and EFI strings is conveted into utf-8. Fixes: a3d0aa87acbe ("bootmenu: update bootmenu_entry structure") Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> Tested-by: Pali Rohar <pali@kernel.org> --- (no change since v1) cmd/bootmenu.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-)