On 09/20/2012 03:32 PM, Peter Krempa wrote:
> On 09/20/12 15:15, Martin Kletzander wrote:
>> On 09/20/2012 12:22 PM, Peter Krempa wrote:
>>>> @@ -8271,6 +8286,19 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr
>>>> caps,
>>>> qemuParseCommandLineBootDevs(def, token);
>>>> } else if (STRPREFIX(token, "menu=on")) {
>>>> def->os.bootmenu = 1;
>>>> + } else if (STRPREFIX(token, "reboot-timeout=")) {
>>>> + int num;
>>>> + char *endptr = strchr(token, ',');
>>>> + if (virStrToLong_i(token +
>>>> strlen("reboot-timeout="),
>>>> + &endptr, 0, &num) < 0) {
>>>
>>> This doesn't seem ok. You assign endptr somewhere into the string and
>>> then virStrToLong_i rewrites it, but you never check the return value. I
>>> suppose you wanted to check if after the number is converted the next
>>> char is a comma. You need to do the check after virStrToLong_i.
>>>
>>
>> Yes, sorry for that, you're absolutely right. To be sure, is this
>> alright?
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index f8012ec..4821910 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -8271,6 +8286,20 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr
>> caps,
>> qemuParseCommandLineBootDevs(def, token);
>> } else if (STRPREFIX(token, "menu=on")) {
>> def->os.bootmenu = 1;
>> + } else if (STRPREFIX(token, "reboot-timeout=")) {
>> + int num;
>> + char *endptr;
>> + if (virStrToLong_i(token +
>> strlen("reboot-timeout="),
>> + &endptr, 10, &num) < 0) ||
>> + endptr != strchr(token, ',') {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("cannot parse
>> reboot-timeout value"));
>> + goto error;
>> + }
>> + if (num > 65535)
>> + num = 65535;
>
> You should probably check for the lower bound too here.
>
>> + def->os.bios.rt_delay = num;
>> + def->os.bios.rt_set = true;
>> }
>> token = strchr(token, ',');
>> /* This incrementation has to be done here in
>> order to make it
>
> ACK to this with the lower bound check.
>
> Peter
>
>
Thanks, fixed that and one more thing (the endptr may point to end of
the string in which case that makes it valid, "thanks tests"), double
checked and pushed.
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list