On Fri, Jun 19, 2009 at 05:54:47PM +0200, Matthias Bolte wrote:
2009/6/19 Daniel P. Berrange <berrange@redhat.com>:
ACK, looks fine to me - though I'm wondering if you might also want to explicitly turn off support for lists ? IIIRC the VMware config values were only scalars, so for safety you might want the parser to refuse to parse / generate lists.
Daniel
Good advice. I've done this now and attached the patch for it. The first patch got already commited, thanks to Daniel Veillard.
Regards, Matthias
diff --git a/src/conf.c b/src/conf.c index ef1a28f..c51531e 100644 --- a/src/conf.c +++ b/src/conf.c @@ -447,6 +447,11 @@ virConfParseValue(virConfParserCtxtPtr ctxt) if (str == NULL) return(NULL); } else if (CUR == '[') { + if (ctxt->conf->flags & VIR_CONF_FLAG_VMX_FORMAT) { + virConfError(ctxt, VIR_ERR_CONF_SYNTAX, + _("lists not allowed in VMX format")); + return(NULL); + } type = VIR_CONF_LIST; NEXT; SKIP_BLANKS_AND_EOL; @@ -486,6 +491,11 @@ virConfParseValue(virConfParserCtxtPtr ctxt) return(NULL); } } else if (c_isdigit(CUR) || (CUR == '-') || (CUR == '+')) { + if (ctxt->conf->flags & VIR_CONF_FLAG_VMX_FORMAT) { + virConfError(ctxt, VIR_ERR_CONF_SYNTAX, + _("numbers not allowed in VMX format")); + return(NULL); + } if (virConfParseLong(ctxt, &l) < 0) { return(NULL); } @@ -530,7 +540,7 @@ virConfParseName(virConfParserCtxtPtr ctxt) } while ((ctxt->cur < ctxt->end) && (c_isalnum(CUR) || (CUR == '_') || - ((ctxt->conf->flags & VIR_CONF_FLAG_ALLOW_VMX_NAMES) && + ((ctxt->conf->flags & VIR_CONF_FLAG_VMX_FORMAT) && ((CUR == ':') || (CUR == '.'))))) NEXT; ret = strndup(base, ctxt->cur - base); diff --git a/src/conf.h b/src/conf.h index 4bb825a..40d9586 100644 --- a/src/conf.h +++ b/src/conf.h @@ -25,8 +25,9 @@ typedef enum { } virConfType;
typedef enum { - VIR_CONF_FLAG_ALLOW_VMX_NAMES = 1, /* allow : and . in names for compatibility - with VMware VMX configuration file */ + VIR_CONF_FLAG_VMX_FORMAT = 1, /* allow : and . in names for compatibility with + VMware VMX configuration file, but restrict + allowed value types to string only */ } virConfFlags;
static inline const char *
ACK, this looks good too. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|