| Message ID | 1541052594-29132-1-git-send-email-liq3ea@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fw_cfg reboot_time and splash_time fix | expand |
> - /* 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); > } Hmm, values > 0xffff are reported and values < 0 are silently ignored. I think we should be consistent here. I'd suggest report and exit in both cases, i.e. use error_setg(..., &error_fatal); cheers, Gerd
Gerd Hoffmann <kraxel@redhat.com> writes: >> - /* 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); >> } > > Hmm, values > 0xffff are reported and values < 0 are silently ignored. > I think we should be consistent here. > I'd suggest report and exit in both cases, Agreed. If the user specifies a value outside acceptable limits, rejecting it is simpler than "correcting" it. "Corrections" may look convenient, but they're not worth the additional interface complexity. > i.e. use error_setg(..., &error_fatal); In case you mean something like error_setg(&error_fatal, "reboot timeout is larger than 65535"); I'd like to point to error.h: * Please don't error_setg(&error_fatal, ...), use error_report() and * exit(), because that's more obvious.
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 3fcfa35..dff6e06 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)
This can avoid setting a negative value to etc/boot-fail-wait. Signed-off-by: Li Qiang <liq3ea@gmail.com> --- hw/nvram/fw_cfg.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)