On Mon, Oct 04, 2010 at 12:34:16PM -0600, Eric Blake wrote:
> On 10/01/2010 09:01 AM, Daniel Veillard wrote:
> >>- if (virXPathULong("string(./vcpu[1])", ctxt,&def->vcpus)< 0)
> >>- def->vcpus = 1;
> >>+ if (virXPathULong("string(./vcpu[1])", ctxt,&count)< 0)
> >>+ def->maxvcpus = 1;
> >>+ else {
> >>+ def->maxvcpus = count;
> >>+ if (def->maxvcpus != count || count == 0) {
> >>+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> >>+ _("invalid maxvcpus %lu"), count);
> >>+ goto error;
> >>+ }
> >>+ }
> >
> > Hum, virXPathULong will return -2 for an non ULong format, and we
> >discard the error by just setting up maxvcpus = 1 silently but on the
> >other hand we make a fuss about 0 being provided :-)
> > If we start raising an error on invalid values maybe it should be
> >done for both (-2 need to be checked)
>
> Which is better? Relying on the .rng file for error checking (in
> which case, XML has already been validated, so not only do we know
> virXPathULong would never return -2, but we also know that
> current='0' would fail validation, so v2 should drop the redundant
> check for 0), or repeat all error checking in the C code (in which
> case adding a check for virXPathULong == -2 is a good thing for v2)?
We don't rely on RNG validation at runtime (the RNG fails on old
libxml2 versions for example), so best to do the checking at parsing
time, but I don't consider this urgent, it's just an improvement
over the current code :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list