Re: [libvirt] [PATCH v2 2/3] qemu: Add support for reboot-timeout

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