On 04/13/2018 02:14 AM, Thomas Huth wrote: > On 10.04.2018 17:01, Collin Walling wrote: >> zIPL boot menu entries can be non-sequential. Let's account >> for this issue for the s390 zIPL boot menu. Since this boot >> menu is actually an imitation and is not completely capable >> of everything the real zIPL menu can do, let's also print a >> different banner to the user. >> >> Signed-off-by: Collin Walling <wall...@linux.ibm.com> >> Reported-by: Vasily Gorbik <g...@linux.ibm.com> >> --- >> pc-bios/s390-ccw/menu.c | 32 ++++++++++++++++++++------------ >> 1 file changed, 20 insertions(+), 12 deletions(-) >> >> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c >> index 96eec81..083405f 100644 >> --- a/pc-bios/s390-ccw/menu.c >> +++ b/pc-bios/s390-ccw/menu.c >> @@ -158,7 +158,7 @@ static void boot_menu_prompt(bool retry) >> } >> } >> >> -static int get_boot_index(int entries) >> +static int get_boot_index(bool *valid_entries) >> { >> int boot_index; >> bool retry = false; >> @@ -168,7 +168,8 @@ static int get_boot_index(int entries) >> boot_menu_prompt(retry); >> boot_index = get_index(); >> retry = true; >> - } while (boot_index < 0 || boot_index >= entries); >> + } while (boot_index < 0 || boot_index >= MAX_BOOT_ENTRIES || >> + !valid_entries[boot_index]); >> >> sclp_print("\nBooting entry #"); >> sclp_print(uitoa(boot_index, tmp, sizeof(tmp))); >> @@ -176,21 +177,28 @@ static int get_boot_index(int entries) >> return boot_index; >> } >> >> -static void zipl_println(const char *data, size_t len) >> +static void zipl_println(const char *data, size_t len, bool *valid_entries) >> { >> char buf[len + 2]; >> + int entry; >> >> ebcdic_to_ascii(data, buf, len); >> buf[len] = '\n'; >> buf[len + 1] = '\0'; >> >> sclp_print(buf); >> + >> + entry = buf[0] == ' ' ? atoui(buf + 1) : atoui(buf); >> + valid_entries[entry] = true; > > zipl_println is now doing more than its name suggests - it now also > populates the valid_entries array. So I think you should either put an > explaining comment in front of zipl_println, or (what I'd prefer), move > this valid_entries populating code rather to the while loop in > menu_get_zipl_boot_index below instead.
Fair enough. I'll spin up v2 for the list after this change and some reassurance testing. Thanks for the feedback and r-b's. > >> + if (entry == 0) >> + sclp_print("\n"); >> } >> >> int menu_get_zipl_boot_index(const char *menu_data) >> { >> size_t len; >> - int entries; >> + bool valid_entries[MAX_BOOT_ENTRIES] = {false}; >> uint16_t zipl_flag = *(uint16_t *)(menu_data - ZIPL_FLAG_OFFSET); >> uint16_t zipl_timeout = *(uint16_t *)(menu_data - ZIPL_TIMEOUT_OFFSET); >> >> @@ -202,19 +210,19 @@ int menu_get_zipl_boot_index(const char *menu_data) >> timeout = zipl_timeout * 1000; >> } >> >> - /* Print and count all menu items, including the banner */ >> - for (entries = 0; *menu_data; entries++) { >> + /* Print banner */ >> + sclp_print("s390-ccw zIPL Boot Menu\n\n"); >> + menu_data += strlen(menu_data) + 1; >> + >> + /* Print entries */ >> + while (*menu_data) { >> len = strlen(menu_data); >> - zipl_println(menu_data, len); >> + zipl_println(menu_data, len, valid_entries); >> menu_data += len + 1; >> - >> - if (entries < 2) { >> - sclp_print("\n"); >> - } >> } >> >> sclp_print("\n"); >> - return get_boot_index(entries - 1); /* subtract 1 to exclude banner */ >> + return get_boot_index(valid_entries); >> } > > Thomas > -- Respectfully, - Collin Walling