[OpenBIOS] [PATCH] arch/ppc/qemu: do not override boot-device if already set

[BALATON Zoltan]

On Sun, 18 May 2014, Mark Cave-Ayland wrote:
> On 18/05/14 00:18, BALATON Zoltan wrote:
>> Signed-off-by: BALATON Zoltan <balaton at eik.bme.hu>
>
> Obfuscation? ;)

I've copied it from the web archives (I still can't subscribe to the list 
by the way). I will correct it in the next version.

>
>> Index: openbios-devel/arch/ppc/qemu/init.c
>> ===================================================================
>> --- openbios-devel/arch/ppc/qemu/init.c	(revision 1298)
>> +++ openbios-devel/arch/ppc/qemu/init.c	(working copy)
>> @@ -659,7 +689,7 @@
>>       char buf[64], qemu_uuid[16];
>>       const char *stdin_path, *stdout_path, *boot_path;
>>       uint32_t temp = 0;
>> -
>> +    char *boot_device;
>>       ofmem_t *ofmem = ofmem_arch_get_private();
>>
>>       openbios_init();
>> @@ -867,24 +903,28 @@
>>       push_str("/options");
>>       fword("find-device");
>> 
>> -    uint16_t boot_device = fw_cfg_read_i16(FW_CFG_BOOT_DEVICE);
>> -    switch (boot_device) {
>> -        case 'c':
>> -            boot_path = "hd";
>> -            break;
>> -        default:
>> -        case 'd':
>> -            boot_path = "cd";
>> -            break;
>> +    /* Setup default boot devices (not overriding user settings) */
>> +    fword("boot-device");
>> +    boot_device = pop_fstr_copy();
>> +    if (strcmp(boot_device, "disk") == 0) {
>
> What is boot-device normally set to if no override is specified?

By default it's set to "disk" in forth/admin/nvram.fs I think.

> pop_fstr_copy() has a little gotcha in that a zero length string becomes a 
> NULL on conversion, so generally I tend to use a pattern like this:
>
>   if (boot_device && strcmp(boot_device, "disk") == 0) {
>       ...
>   }

I did not test that the user can set it to "" but if so I agree it's 
better to check for a NULL value. I'll test this and send a v2.

> I'm just wondering if we shouldn't always assume that a value exists and is 
> non-zero...

So you mean that the default value should also be set when it's empty not 
only when not set explicitely? Now I wonder if the default should be 
changed to empty instead but I don't know what else would that affect.

Regards,
BALATON Zoltan