[SeaBIOS] [PATCH] add F11 shortcut for network boot

Paolo Bonzini pbonzini at redhat.com
Wed Oct 17 17:03:32 CEST 2012


Il 16/10/2012 15:36, Peter Stuge ha scritto:
> Paolo Bonzini wrote:
>> +++ b/src/boot.c
>> @@ -400,54 +400,75 @@ interactive_bootmenu(void)
>>      while (get_keystroke(0) >= 0)
>>          ;
>>  
>> -    printf("Press F12 for boot menu.\n\n");
>> +    wait_threads();
>> +    struct bootentry_s *pos;
>> +    for (pos = BootList; pos; pos = pos->next) {
>> +        if (pos->type > IPL_TYPE_CBFS)
>> +            break;
>> +    }
>> +
>> +    printf("Press %sF12 for boot menu.\n\n"
>> +           , pos ? "F11 for network boot, or " : "");
> 
> This printf is not the prettiest code I've seen. Maybe there is a
> nicer way to accomplish the same output?

Any suggestion?

>> -    if (scan_code != 0x86)
>> +    if (scan_code != 0x86 && (scan_code != 0x85 || !pos))
>>          /* not F12 */
>>          return;
> 
> Did you read the comment? After your change the comment is extremely
> confusing. Why don't you also change the comment when you change the
> code.

Oops.

>> +    if (scan_code == 0x85) {
> 
> How about using switch() ?

Ok.

>> +        // Prioritize BEV/BCV entries
>> +        struct bootentry_s **pprev = &BootList, **pprev_new = &BootList;
>> +        while ((pos = *pprev) != NULL) {
>> +            if (pos->type > IPL_TYPE_CBFS) {
>> +                *pprev = pos->next;
>> +                pos->next = *pprev_new;
>> +                *pprev_new = pos;
>> +                pos->priority = 0;
>> +                pprev_new = &pos->next;
>> +            } else {
>> +                pprev = &pos->next;
>> +            }
>> +        }
> 
> The above could be written as a very nice and readable for () loop
> instead. Handle the uninteresting case with continue.

No, it cannot.  One branch sets pprev_new, the other sets pprev.

>> +        // Get key press
>> +        for (;;) {
>> +            scan_code = get_keystroke(1000);
>> +            if (scan_code >= 1 && scan_code <= maxmenu+1)
>> +                break;
>> +        }
>> +        printf("\n");
>> +        if (scan_code == 0x01)
>> +            // ESC
>> +            return;
> 
> Maybe it's time to refactor some of this code at this point, instead
> of copypasting it.

This is not copy-paste, it's reindentation.  I can split it in two
patches though to make it clearer.

Paolo




More information about the SeaBIOS mailing list