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

Mark Cave-Ayland mark.cave-ayland at ilande.co.uk
Sun May 18 13:18:41 CEST 2014


On 18/05/14 00:18, BALATON Zoltan wrote:

> Only set the boot-device env variable if it is not yet set, do not
> override values set e.g. by -prom-env option of QEMU.
>
> Signed-off-by: BALATON Zoltan <balaton at eik.bme.hu>

Obfuscation? ;)

> 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? 
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'm just wondering if we shouldn't always assume that a value exists and 
is non-zero...

> +        switch (fw_cfg_read_i16(FW_CFG_BOOT_DEVICE)) {
> +            case 'c':
> +                boot_path = "hd";
> +                break;
> +            default:
> +            case 'd':
> +                boot_path = "cd";
> +                break;
> +        }
> +
> +        snprintf(buf, sizeof(buf), "%s:,\\\\:tbxi %s:,\\ppc\\bootinfo.txt %s:,%%BOOT", boot_path, boot_path, boot_path);
> +        push_str(buf);
> +        fword("encode-string");
> +        push_str("boot-device");
> +        fword("property");
>       }
> +    free(boot_device);
>
> -    /* Setup default boot devices */
> -    snprintf(buf, sizeof(buf), "%s:,\\\\:tbxi %s:,\\ppc\\bootinfo.txt %s:,%%BOOT", boot_path, boot_path, boot_path);
> -    push_str(buf);
> -    fword("encode-string");
> -    push_str("boot-device");
> -    fword("property");
> -
>       /* Set up other properties */
>
>       push_str("/chosen");

Other than that, looks reasonable to me and I'd be okay to apply this 
unless someone vociferously objects.


ATB,

Mark.




More information about the OpenBIOS mailing list