[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] hw: fw_cfg: ensure reboot_time is nonegativ
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] hw: fw_cfg: ensure reboot_time is nonegative |
Date: |
Mon, 05 Nov 2018 16:50:29 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Gerd Hoffmann <address@hidden> 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.