[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