On 03/31/2010 03:41 PM, Matthias Bolte wrote: > 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. > +++ b/src/esx/esx_driver.c > @@ -684,34 +684,17 @@ static int > esxGetVersion(virConnectPtr conn, unsigned long *version) > { ... > + 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); String needs translation. > 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) ... > + /* expected format: vzctl version <major>.<minor>.<micro> */ > + if ((tmp = STRSKIP(tmp, "vzctl version ")) == NULL) > goto cleanup2; Yes, that looks nicer. Glad I did my thinking out loud, and for other's input on the name. > +++ 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; Coreutils uses an idiom bad_cast(str) to make it obvious that we are casting away the const, and that we have audited that the results are still sane (virStrToLong_ui does not modify its argument), while at the same time reducing the number of C-style casts that require you to think why we are casting. But introducing bad_cast() into libvirt would be a separate patch; your code is fine as-is for this round. ACK with the translation markup. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list