| Message ID | de5f19510376871e9a56b8704e3943927c8c65c5.1397794966.git.peter.crosthwaite@xilinx.com |
|---|---|
| State | New |
| Headers | show |
Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes: > When accessing boot options, we query whatever options come first in > the boot opts list. This is wrong. You could copy a bit more text from commit 36ad0e9 to explain how exactly it is wrong. First three paragraphs, perhaps. > Use qemu_get_boot_opts() to fix these bugs. > > This change is similar to and based on 36ad0e9. > > We also take to opportunity to remove the now unneeded null boot-opts > conditional, removing a level of indentation on usage code. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > > hw/nvram/fw_cfg.c | 36 ++++++++++++++++-------------------- > 1 file changed, 16 insertions(+), 20 deletions(-) > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 282341a..8537669 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -125,18 +125,16 @@ static void fw_cfg_bootsplash(FWCfgState *s) > const char *temp; > > /* get user configuration */ > - QemuOptsList *plist = qemu_find_opts("boot-opts"); > - QemuOpts *opts = QTAILQ_FIRST(&plist->head); > - if (opts != NULL) { > - temp = qemu_opt_get(opts, "splash"); > - if (temp != NULL) { > - boot_splash_filename = temp; > - } > - temp = qemu_opt_get(opts, "splash-time"); > - if (temp != NULL) { > - p = (char *)temp; > - boot_splash_time = strtol(p, (char **)&p, 10); > - } > + QemuOpts *opts = qemu_get_boot_opts(); > + > + temp = qemu_opt_get(opts, "splash"); > + if (temp != NULL) { > + boot_splash_filename = temp; > + } > + temp = qemu_opt_get(opts, "splash-time"); > + if (temp != NULL) { > + p = (char *)temp; > + boot_splash_time = strtol(p, (char **)&p, 10); > } > > /* insert splash time if user configurated */ Separate issue, and I'm not demanding you fix it: strtol() can fail. Parameter "splash-time" should be QEMU_OPT_NUMBER, and it should be gotten with qemu_opt_get_number(). > @@ -191,14 +189,12 @@ static void fw_cfg_reboot(FWCfgState *s) > const char *temp; > > /* get user configuration */ > - QemuOptsList *plist = qemu_find_opts("boot-opts"); > - QemuOpts *opts = QTAILQ_FIRST(&plist->head); > - if (opts != NULL) { > - temp = qemu_opt_get(opts, "reboot-timeout"); > - if (temp != NULL) { > - p = (char *)temp; > - reboot_timeout = strtol(p, (char **)&p, 10); > - } > + QemuOpts *opts = qemu_get_boot_opts(); > + > + temp = qemu_opt_get(opts, "reboot-timeout"); > + if (temp != NULL) { > + p = (char *)temp; > + reboot_timeout = strtol(p, (char **)&p, 10); > } > /* validate the input */ > if (reboot_timeout > 0xffff) { Likewise. Patch looks good. Let's wait a few days for qemu_find_opts_singleton(), okay?
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 282341a..8537669 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -125,18 +125,16 @@ static void fw_cfg_bootsplash(FWCfgState *s) const char *temp; /* get user configuration */ - QemuOptsList *plist = qemu_find_opts("boot-opts"); - QemuOpts *opts = QTAILQ_FIRST(&plist->head); - if (opts != NULL) { - temp = qemu_opt_get(opts, "splash"); - if (temp != NULL) { - boot_splash_filename = temp; - } - temp = qemu_opt_get(opts, "splash-time"); - if (temp != NULL) { - p = (char *)temp; - boot_splash_time = strtol(p, (char **)&p, 10); - } + QemuOpts *opts = qemu_get_boot_opts(); + + temp = qemu_opt_get(opts, "splash"); + if (temp != NULL) { + boot_splash_filename = temp; + } + temp = qemu_opt_get(opts, "splash-time"); + if (temp != NULL) { + p = (char *)temp; + boot_splash_time = strtol(p, (char **)&p, 10); } /* insert splash time if user configurated */ @@ -191,14 +189,12 @@ static void fw_cfg_reboot(FWCfgState *s) const char *temp; /* get user configuration */ - QemuOptsList *plist = qemu_find_opts("boot-opts"); - QemuOpts *opts = QTAILQ_FIRST(&plist->head); - if (opts != NULL) { - temp = qemu_opt_get(opts, "reboot-timeout"); - if (temp != NULL) { - p = (char *)temp; - reboot_timeout = strtol(p, (char **)&p, 10); - } + QemuOpts *opts = qemu_get_boot_opts(); + + temp = qemu_opt_get(opts, "reboot-timeout"); + if (temp != NULL) { + p = (char *)temp; + reboot_timeout = strtol(p, (char **)&p, 10); } /* validate the input */ if (reboot_timeout > 0xffff) {
When accessing boot options, we query whatever options come first in the boot opts list. This is wrong. Use qemu_get_boot_opts() to fix these bugs. This change is similar to and based on 36ad0e9. We also take to opportunity to remove the now unneeded null boot-opts conditional, removing a level of indentation on usage code. Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- hw/nvram/fw_cfg.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-)