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.
+ } 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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org