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

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.