[libvirt] [PATCH] esx: Map the .vmx annotation to the domain XML description

[Eric Blake]

On 08/27/2010 10:31 AM, Matthias Bolte wrote:
> +    char *annotation = NULL;
> +    char *tmp;
> +    int length;

s/int/size_t/

>       int controller;
>       int bus;
>       int port;
> @@ -947,6 +950,33 @@ esxVMX_ParseConfig(esxVMX_Context *ctx, virCapsPtr caps, const char *vmx,
>           goto cleanup;
>       }
>
> +    /* vmx:annotation ->  def:description */
> +    if (esxUtil_GetConfigString(conf, "annotation",&def->description,
> +                                true)<  0) {
> +        goto cleanup;
> +    }
> +
> +    /* Replace '|22' with '"' and '|7C' with '|' */

Looking at that comment, it looks like |xx for any two arbitrary hex 
digits is a valid escape sequence.  While you only need to be strict in 
what you generate (escapes only for the problematic characters that 
can't be sent literally), should you be liberal in what you accept (all 
possible | escapes for any byte, even if the byte could have been sent 
literally)?

> +    if (def->description != NULL) {
> +        length = strlen(def->description) + 1;
> +        tmp = def->description;
> +
> +        while (*tmp != '\0') {
> +            if (STRPREFIX(tmp, "|22")) {
> +                *tmp = '"';
> +                memmove(tmp + 1, tmp + 3, length - 3);
> +                length -= 2;

Hmm - this scales quadratically (that is, decoding a sequence of 20 
"|22" requires 19 memmoves, totaling 190 sequence relocations).  I would 
prefer a linear rewrite - have two pointers, one that tracks where you 
are reading, and one that tracks where you are writing (you already did 
this on the loop adding the escapes).  Then, on every iteration, the 
read pointer may advance multiple positions, but the write pointer 
advances only one, and you don't need any memmoves.

> +            } else if (STRPREFIX(tmp, "|7C") || STRPREFIX(tmp, "|7c")) {
> +                *tmp = '|';

Dead assignment since we already know *tmp is '|'.  Then again, if you 
change to accept all escapes, this line would disappear anyways.

> @@ -2314,10 +2345,15 @@ char *
>   esxVMX_FormatConfig(esxVMX_Context *ctx, virCapsPtr caps, virDomainDefPtr def,
>                       esxVI_ProductVersion productVersion)
>   {
> +    char *vmx = NULL;
>       int i;
>       int sched_cpu_affinity_length;
>       unsigned char zero[VIR_UUID_BUFLEN];
>       virBuffer buffer = VIR_BUFFER_INITIALIZER;
> +    int length;

s/int/size_t/

The rest of the patch looks okay to me, however.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org