On 10/24/18 09:11, Li Qiang wrote: > This can avoid setting a negative value to > etc/boot-fail-wait. > > Signed-off-by: Li Qiang <liq...@gmail.com> > --- > hw/nvram/fw_cfg.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index f4a52d8..276dcb1 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -199,12 +199,17 @@ static void fw_cfg_reboot(FWCfgState *s) > reboot_timeout = strtol(p, &p, 10); > } > } > - /* validate the input */ > - if (reboot_timeout > 0xffff) { > - error_report("reboot timeout is larger than 65535, force it to > 65535."); > - reboot_timeout = 0xffff; > + > + if (reboot_timeout >= 0) { > + /* validate the input */ > + if (reboot_timeout > 0xffff) { > + error_report("reboot timeout is larger than 65535," > + "force it to 65535."); > + reboot_timeout = 0xffff; > + } > + fw_cfg_add_file(s, "etc/boot-fail-wait", > + g_memdup(&reboot_timeout, 4), 4); > } > - fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(&reboot_timeout, 4), > 4); > } > > static void fw_cfg_write(FWCfgState *s, uint8_t value) >
I don't feel strongly about fixing this issue. However, if we decide to fix it, we should start with the bare-bones strtol() call, visible at the top of the context. I'm not up-to-date on what's the best QEMU helper function for this, but I seem to remember it checks for trailing garbage, and perhaps even for range. Maybe we should even use a different (better) option parsing facility thatn qemu_opt_get(). Adding Eric and Markus. Also, I would suggest forcing negative values (that were explicitly specified) to some sensible positive default, such as 5 seconds or so. Thanks Laszlo