[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/3] Re-factor qemu version parsing



On Wed, Jun 10, 2009 at 09:02:39PM +0100, Mark McLoughlin wrote:
> This patch is purely re-factoring without any functional changes
> to make way for the next patch.
> 
> The main thing achieved by the refactoring is that we now have
> easier access to the parenthesised string that KVM folks seem
> to delight in changing.

  This kind of parsing looks more flexible to me, even if a bit
less readable than using pure sscanf

[...]
> +#define QEMU_VERSION_STR    "QEMU PC emulator version "
> +#define KVM_VER_PREFIX      " (kvm-"
> +
> +static int qemudParseHelpStr(const char *help,
> +                             unsigned int *flags,
> +                             unsigned int *version,
> +                             unsigned int *kvm_version)
> +{
> +    unsigned major, minor, micro;
> +    const char *p = help;
> +
> +    *flags = *version = *kvm_version = 0;
> +
> +    if (!STRPREFIX(p, QEMU_VERSION_STR))
> +        goto fail;
> +
> +    p += strlen(QEMU_VERSION_STR);

  But I would not integate the spaces in the strings and instead
use a macro like
 #define QEMU_VERSION_STR    "QEMU PC emulator version"
 #define SKIP_BLANKS(p) while ((*(p) == ' ') || (*(p) == '\t')) (p)++;

 and here do SKIP_BLANKS(p)

> +    major = virParseNumber(&p);
> +    if (major == -1 || *p != '.')
> +        goto fail;
> +
> +    ++p;
> +
> +    minor = virParseNumber(&p);
> +    if (major == -1 || *p != '.')
> +        goto fail;
> +
> +    ++p;
> +
> +    micro = virParseNumber(&p);
> +    if (major == -1)
> +        goto fail;

  and SKIP_BLANKS(p) here
and
  #define KVM_VER_PREFIX      "(kvm-"
without the leading space.

> +    if (STRPREFIX(p, KVM_VER_PREFIX)) {
> +        int ret;
> +
> +        p += strlen(KVM_VER_PREFIX);
> +
> +        ret = virParseNumber(&p);
> +        if (ret == -1)
> +            goto fail;
> +
> +        *kvm_version = ret;
[...]

  But I'm biased on parsing ... and that's not critical, ACK,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]