| Message ID | 20230406140333.1529863-1-trini@konsulko.com |
|---|---|
| State | Accepted |
| Commit | e0dda26c2e9fb4150a5ab0b8b5ad58da5dd355cd |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | bootflow: Rework do_bootflow_menu() slightly | expand |
On Fri, 7 Apr 2023 at 02:03, Tom Rini <trini@konsulko.com> wrote: > > When building this with clang, we get a warning such as: > cmd/bootflow.c:412:27: warning: variable 'bflow' is uninitialized when used here [-Wuninitialized] > printf("Selected: %s\n", bflow->os_name ? bflow->os_name : bflow->name); > ^~~~~ > > And a suggestion to just initialize bflow to NULL. This would however > would be ensuring a bad dereference. Instead, looking at the function we > rework things so that when CONFIG_EXPO is not enabled (and so, no UI) we > error early and would never reach this point in the code. Simplify the > rest slightly as well while at this. > > Signed-off-by: Tom Rini <trini@konsulko.com> > --- > Cc: Simon Glass <sjg@chromium.org> > --- > cmd/bootflow.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org>
On Fri, 7 Apr 2023 at 02:03, Tom Rini <trini@konsulko.com> wrote: > > When building this with clang, we get a warning such as: > cmd/bootflow.c:412:27: warning: variable 'bflow' is uninitialized when used here [-Wuninitialized] > printf("Selected: %s\n", bflow->os_name ? bflow->os_name : bflow->name); > ^~~~~ > > And a suggestion to just initialize bflow to NULL. This would however > would be ensuring a bad dereference. Instead, looking at the function we > rework things so that when CONFIG_EXPO is not enabled (and so, no UI) we > error early and would never reach this point in the code. Simplify the > rest slightly as well while at this. > > Signed-off-by: Tom Rini <trini@konsulko.com> > --- > Cc: Simon Glass <sjg@chromium.org> > --- > cmd/bootflow.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> Applied to u-boot-dm, thanks!
diff --git a/cmd/bootflow.c b/cmd/bootflow.c index 42f6e14a4370..2c3889a6f0de 100644 --- a/cmd/bootflow.c +++ b/cmd/bootflow.c @@ -387,6 +387,11 @@ static int do_bootflow_menu(struct cmd_tbl *cmdtp, int flag, int argc, bool text_mode = false; int ret; + if (!IS_ENABLED(CONFIG_EXPO)) { + printf("Menu not supported\n"); + return CMD_RET_FAILURE; + } + if (argc > 1 && *argv[1] == '-') text_mode = strchr(argv[1], 't'); @@ -394,20 +399,15 @@ static int do_bootflow_menu(struct cmd_tbl *cmdtp, int flag, int argc, if (ret) return CMD_RET_FAILURE; - if (IS_ENABLED(CONFIG_EXPO)) { - ret = bootflow_menu_run(std, text_mode, &bflow); - if (ret) { - if (ret == -EAGAIN) - printf("Nothing chosen\n"); - else - printf("Menu failed (err=%d)\n", ret); + ret = bootflow_menu_run(std, text_mode, &bflow); + if (ret) { + if (ret == -EAGAIN) + printf("Nothing chosen\n"); + else { + printf("Menu failed (err=%d)\n", ret); + return CMD_RET_FAILURE; } - } else { - printf("Menu not supported\n"); - ret = -ENOSYS; } - if (ret) - return CMD_RET_FAILURE; printf("Selected: %s\n", bflow->os_name ? bflow->os_name : bflow->name); std->cur_bootflow = bflow;
When building this with clang, we get a warning such as: cmd/bootflow.c:412:27: warning: variable 'bflow' is uninitialized when used here [-Wuninitialized] printf("Selected: %s\n", bflow->os_name ? bflow->os_name : bflow->name); ^~~~~ And a suggestion to just initialize bflow to NULL. This would however would be ensuring a bad dereference. Instead, looking at the function we rework things so that when CONFIG_EXPO is not enabled (and so, no UI) we error early and would never reach this point in the code. Simplify the rest slightly as well while at this. Signed-off-by: Tom Rini <trini@konsulko.com> --- Cc: Simon Glass <sjg@chromium.org> --- cmd/bootflow.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)