Parsing is stricter now and doesn't accept trailing characters after the actual <domain>:<bus>:<slot> sequence anymore. --- src/conf/domain_conf.c | 35 +++++++++++++++++++++++------------ 1 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bf4e657..0ad2e4a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1290,6 +1290,23 @@ cleanup: return ret; } +static int +virDomainParseLegacyDeviceAddress(char *devaddr, + virDomainDevicePCIAddressPtr pci) +{ + char *tmp; + + /* expected format: <domain>:<bus>:<slot> */ + if (/* domain */ + virStrToLong_ui(devaddr, &tmp, 16, &pci->domain) < 0 || *tmp != ':' || + /* bus */ + virStrToLong_ui(tmp + 1, &tmp, 16, &pci->bus) < 0 || *tmp != ':' || + /* slot */ + virStrToLong_ui(tmp + 1, NULL, 16, &pci->slot) < 0) + return -1; + + return 0; +} int virDomainDiskDefAssignAddress(virDomainDiskDefPtr def) @@ -1541,10 +1558,8 @@ virDomainDiskDefParseXML(xmlNodePtr node, } if (devaddr) { - if (sscanf(devaddr, "%x:%x:%x", - &def->info.addr.pci.domain, - &def->info.addr.pci.bus, - &def->info.addr.pci.slot) < 3) { + if (virDomainParseLegacyDeviceAddress(devaddr, + &def->info.addr.pci) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse devaddr parameter '%s'"), devaddr); @@ -1901,10 +1916,8 @@ virDomainNetDefParseXML(virCapsPtr caps, } if (devaddr) { - if (sscanf(devaddr, "%x:%x:%x", - &def->info.addr.pci.domain, - &def->info.addr.pci.bus, - &def->info.addr.pci.slot) < 3) { + if (virDomainParseLegacyDeviceAddress(devaddr, + &def->info.addr.pci) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse devaddr parameter '%s'"), devaddr); @@ -3222,10 +3235,8 @@ virDomainHostdevSubsysPciDefParseXML(const xmlNodePtr node, /* Legacy back-compat. Don't add any more attributes here */ char *devaddr = virXMLPropString(cur, "devaddr"); if (devaddr && - sscanf(devaddr, "%x:%x:%x", - &def->info.addr.pci.domain, - &def->info.addr.pci.bus, - &def->info.addr.pci.slot) < 3) { + virDomainParseLegacyDeviceAddress(devaddr, + &def->info.addr.pci) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse devaddr parameter '%s'"), devaddr); -- 1.6.3.3

Parsing is stricter now and doesn't accept trailing characters after the actual value anymore. --- src/conf/nwfilter_conf.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 876b3ef..fbf6f33 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -1239,7 +1239,7 @@ virNWFilterRuleDetailsParse(virConnectPtr conn ATTRIBUTE_UNUSED, case DATATYPE_UINT8: storage_ptr = &item->u.u8; - if (sscanf(prop, "%d", &int_val) == 1) { + if (virStrToLong_i(prop, NULL, 10, &int_val) >= 0) { if (int_val >= 0 && int_val <= 0xff) { if (!validator) *(uint8_t *)storage_ptr = int_val; @@ -1253,7 +1253,7 @@ virNWFilterRuleDetailsParse(virConnectPtr conn ATTRIBUTE_UNUSED, case DATATYPE_UINT16: storage_ptr = &item->u.u16; - if (sscanf(prop, "%d", &int_val) == 1) { + if (virStrToLong_i(prop, NULL, 10, &int_val) >= 0) { if (int_val >= 0 && int_val <= 0xffff) { if (!validator) *(uint16_t *)storage_ptr = int_val; @@ -1277,7 +1277,7 @@ virNWFilterRuleDetailsParse(virConnectPtr conn ATTRIBUTE_UNUSED, case DATATYPE_IPMASK: storage_ptr = &item->u.u8; if (!virNWIPv4AddressParser(prop, &ipaddr)) { - if (sscanf(prop, "%d", &int_val) == 1) { + if (virStrToLong_i(prop, NULL, 10, &int_val) >= 0) { if (int_val >= 0 && int_val <= 32) { if (!validator) *(uint8_t *)storage_ptr = @@ -1331,7 +1331,7 @@ virNWFilterRuleDetailsParse(virConnectPtr conn ATTRIBUTE_UNUSED, case DATATYPE_IPV6MASK: storage_ptr = &item->u.u8; if (!virNWIPv6AddressParser(prop, &ipaddr)) { - if (sscanf(prop, "%d", &int_val) == 1) { + if (virStrToLong_i(prop, NULL, 10, &int_val) >= 0) { if (int_val >= 0 && int_val <= 128) { if (!validator) *(uint8_t *)storage_ptr = @@ -1659,7 +1659,7 @@ virNWFilterRuleParse(virConnectPtr conn, ret->priority = MAX_RULE_PRIORITY / 2; if (prio) { - if (sscanf(prio, "%d", (int *)&priority) == 1) { + if (virStrToLong_i(prio, NULL, 10, (int *)&priority) >= 0) { if ((int)priority >= 0 && priority <= MAX_RULE_PRIORITY) ret->priority = priority; } -- 1.6.3.3

virParseVersionString uses virStrToLong_ui instead of sscanf. This also fixes a bug in the UML driver, that always returned 0 as version number. Introduce STRSKIP to check if a string has a certain prefix and to skip this prefix. --- src/esx/esx_driver.c | 29 ++++++----------------------- src/internal.h | 1 + src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 10 ++-------- src/openvz/openvz_conf.c | 14 ++++++++------ src/uml/uml_conf.h | 2 +- src/uml/uml_driver.c | 17 +++++++++-------- src/util/util.c | 35 +++++++++++++++++++++++++++++++++++ src/util/util.h | 1 + src/vbox/vbox_tmpl.c | 14 ++++---------- src/xenapi/xenapi_driver.c | 20 +++++++++----------- 11 files changed, 77 insertions(+), 67 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 20376e9..7a92c4d 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -684,34 +684,17 @@ static int esxGetVersion(virConnectPtr conn, unsigned long *version) { esxPrivate *priv = conn->privateData; - char *temp; - unsigned int major, minor, release; - temp = (char *)priv->host->service->about->version; - - /* Expecting 'major.minor.release' format */ - if (virStrToLong_ui(temp, &temp, 10, &major) < 0 || *temp != '.') { - goto failure; - } - - if (virStrToLong_ui(temp + 1, &temp, 10, &minor) < 0 || *temp != '.') { - goto failure; - } + if (virParseVersionString(priv->host->service->about->version, + version) < 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + "Could not parse version number from '%s'", + priv->host->service->about->version); - if (virStrToLong_ui(temp + 1, NULL, 10, &release) < 0) { - goto failure; + return -1; } - *version = 1000000 * major + 1000 * minor + release; - return 0; - - failure: - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - "Expecting version to match 'major.minor.release', but got '%s'", - priv->host->service->about->version); - - return -1; } diff --git a/src/internal.h b/src/internal.h index f82fbd2..807288b 100644 --- a/src/internal.h +++ b/src/internal.h @@ -57,6 +57,7 @@ # define STRNEQLEN(a,b,n) (strncmp(a,b,n) != 0) # define STRCASENEQLEN(a,b,n) (strncasecmp(a,b,n) != 0) # define STRPREFIX(a,b) (strncmp(a,b,strlen(b)) == 0) +# define STRSKIP(a,b) (STRPREFIX(a,b) ? (a) + strlen(b) : NULL) # define STREQ_NULLABLE(a, b) \ ((!(a) && !(b)) || ((a) && (b) && STREQ((a), (b)))) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc943f8..edb23c2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -648,6 +648,7 @@ virFilePid; virFileReadPid; virFileLinkPointsTo; virParseNumber; +virParseVersionString; virPipeReadUntilEOF; virAsprintf; virRun; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 7ebc7ae..9caefa1 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1951,20 +1951,14 @@ lxcActive(void) { static int lxcVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *version) { struct utsname ver; - int maj; - int min; - int rev; uname(&ver); - if (sscanf(ver.release, "%i.%i.%i", &maj, &min, &rev) != 3) { - lxcError(VIR_ERR_INTERNAL_ERROR, - _("Unknown release: %s"), ver.release); + if (virParseVersionString(ver.release, version) < 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, _("Unknown release: %s"), ver.release); return -1; } - *version = (maj * 1000 * 1000) + (min * 1000) + rev; - return 0; } diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 3713a45..7fc3cd1 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -78,8 +78,8 @@ openvzExtractVersionInfo(const char *cmd, int *retversion) pid_t child; int newstdout = -1; int ret = -1, status; - unsigned int major, minor, micro; - unsigned int version; + unsigned long version; + char *tmp; if (retversion) *retversion = 0; @@ -93,12 +93,14 @@ openvzExtractVersionInfo(const char *cmd, int *retversion) if (len < 0) goto cleanup2; - if (sscanf(help, "vzctl version %u.%u.%u", - &major, &minor, &micro) != 3) { + tmp = help; + + /* expected format: vzctl version <major>.<minor>.<micro> */ + if ((tmp = STRSKIP(tmp, "vzctl version ")) == NULL) goto cleanup2; - } - version = (major * 1000 * 1000) + (minor * 1000) + micro; + if (virParseVersionString(tmp, &version) < 0) + goto cleanup2; if (retversion) *retversion = version; diff --git a/src/uml/uml_conf.h b/src/uml/uml_conf.h index 70c3f7d..4d434e1 100644 --- a/src/uml/uml_conf.h +++ b/src/uml/uml_conf.h @@ -44,7 +44,7 @@ struct uml_driver { int privileged; - unsigned int umlVersion; + unsigned long umlVersion; int nextvmid; virDomainObjList domains; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 835e5d4..08fbf93 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1225,17 +1225,18 @@ cleanup: static int umlGetVersion(virConnectPtr conn, unsigned long *version) { struct uml_driver *driver = conn->privateData; struct utsname ut; - int major, minor, micro; int ret = -1; - uname(&ut); - umlDriverLock(driver); - if (sscanf(ut.release, "%u.%u.%u", - &major, &minor, &micro) != 3) { - umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("cannot parse version %s"), ut.release); - goto cleanup; + + if (driver->umlVersion == 0) { + uname(&ut); + + if (virParseVersionString(ut.release, &driver->umlVersion) < 0) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot parse version %s"), ut.release); + goto cleanup; + } } *version = driver->umlVersion; diff --git a/src/util/util.c b/src/util/util.c index 62dc5f1..6c48a93 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2074,6 +2074,41 @@ virParseNumber(const char **str) return (ret); } + +/** + * virParseVersionString: + * @str: const char pointer to the version string + * @version: unsigned long pointer to output the version number + * + * Parse an unsigned version number from a version string. Expecting + * 'major.minor.micro' format, ignoring an optional suffix. + * + * The major, minor and micro numbers are encoded into a single version number: + * + * 1000000 * major + 1000 * minor + micro + * + * Returns the 0 for success, -1 for error. + */ +int +virParseVersionString(const char *str, unsigned long *version) +{ + unsigned int major, minor, micro; + char *tmp = (char *)str; + + if (virStrToLong_ui(tmp, &tmp, 10, &major) < 0 || *tmp != '.') + return -1; + + if (virStrToLong_ui(tmp + 1, &tmp, 10, &minor) < 0 || *tmp != '.') + return -1; + + if (virStrToLong_ui(tmp + 1, &tmp, 10, &micro) < 0) + return -1; + + *version = 1000000 * major + 1000 * minor + micro; + + return 0; +} + /** * virAsprintf * diff --git a/src/util/util.h b/src/util/util.h index 24dfbfc..c256117 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -189,6 +189,7 @@ int virMacAddrCompare (const char *mac1, const char *mac2); void virSkipSpaces(const char **str); int virParseNumber(const char **str); +int virParseVersionString(const char *str, unsigned long *version); int virAsprintf(char **strp, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(2, 3); char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) ATTRIBUTE_RETURN_CHECK; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index f7a9b9f..05b075f 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -157,7 +157,7 @@ if (strUtf16) {\ typedef struct { virMutex lock; - int version; + unsigned long version; virCapsPtr caps; @@ -713,10 +713,7 @@ cleanup: } static int vboxExtractVersion(virConnectPtr conn, vboxGlobalData *data) { - unsigned int major = 0; - unsigned int minor = 0; - unsigned int micro = 0; - int ret = -1; + int ret = -1; PRUnichar *versionUtf16 = NULL; nsresult rc; @@ -729,20 +726,17 @@ static int vboxExtractVersion(virConnectPtr conn, vboxGlobalData *data) { VBOX_UTF16_TO_UTF8(versionUtf16, &vboxVersion); - if (sscanf(vboxVersion, "%u.%u.%u", &major, &minor, &micro) == 3) + if (virParseVersionString(vboxVersion, &data->version) >= 0) ret = 0; VBOX_UTF8_FREE(vboxVersion); VBOX_COM_UNALLOC_MEM(versionUtf16); - } else { - ret = -1; } - data->version = (major * 1000 * 1000) + (minor * 1000) + micro; - if (ret != 0) vboxError(conn, VIR_ERR_INTERNAL_ERROR,"%s", "Cound not extract VirtualBox version"); + return ret; } diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index d48bb4d..dcfdc1e 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -253,9 +253,8 @@ xenapiGetVersion (virConnectPtr conn, unsigned long *hvVer) xen_host host; xen_session *session = ((struct _xenapiPrivate *)(conn->privateData))->session; xen_string_string_map *result = NULL; - int i; + int i, ret = -1; char *version = NULL; - unsigned long major = 0, minor = 0, release = 0; if (!(xen_session_get_this_host(session, &host, session))) { xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, NULL); return -1; @@ -278,18 +277,17 @@ xenapiGetVersion (virConnectPtr conn, unsigned long *hvVer) } } if (version) { - if (sscanf(version, "%ld.%ld.%ld", &major, &minor, &release) != 3) { + if (virParseVersionString(version, hvVer) < 0) xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, - _("Couldn't get version info")); - xen_string_string_map_free(result); - VIR_FREE(version); - return -1; - } - *hvVer = major * 1000000 + minor * 1000 + release; - VIR_FREE(version); + _("Couldn't parse version info")); + else + ret = 0; xen_string_string_map_free(result); - return 0; + VIR_FREE(version); + return ret; } + xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, + _("Couldn't get version info")); } return -1; } -- 1.6.3.3

2010/4/1 Eric Blake <eblake(a)redhat.com&gt;: Currently, no string in the ESX driver code is marked for translation. I'll fix that in a separate patch. I can just pass str to the first virStrToLong_ui, as it takes a const char pointer as input. So I can get rid of the bad char pointer cast. As said before I defer that to a separate patch, that covers the whole ESX driver code. Matthias