On 08/05/2010 12:45 PM, Patrick Dignan wrote:
> diff -Naurb libvirt-0.8.2.orig/docs/schemas/storagepool.rng
Ouch - this flattened the whitespace to the point that it makes
reviewing harder to do. Could you consider resending, either via 'git
send-email' or by attaching the output of 'git format-patch', such that
your indentation is preserved?
> libvirt-0.8.2.ml/docs/schemas/storagepool.rng
> --- libvirt-0.8.2.orig/docs/schemas/storagepool.rng 2010-06-16
> 17:27:22.000000000 -0500
> +++ libvirt-0.8.2.ml/docs/schemas/storagepool.rng 2010-08-05
> 12:16:31.703536682 -0500
> @@ -103,6 +103,19 @@
> <ref name='target'/>
> </define>
>
> + <define name='sourceinfovendor'>
> + <element name='vendor'>
> + <attribute name='name'>
> + <text/>
> + </attribute>
> + </element>
> + <element name='product'>
> + <attribute name='name'>
> + <text/>
> + </attribute>
> + </element>
> + </define>
Would it ever make sense to have only vendor or product, rather than to
always require both elements or neither? If so, you either need two
<define>s or some more .rng magic around the two <element>s.
> @@ -465,6 +469,18 @@
> goto cleanup;
> }
>
> + vendor = virXPathString("string(./vendor/@name)", ctxt);
> +
> + if (vendor != NULL) {
> + source->vendor = strdup(vendor);
virXPathString returns malloc'd memory. No need to strdup it; just use
it as is - that will avoid a memory leak, since you didn't call
VIR_FREE(vendor) in the cleanup: label.
> + }
> +
> + product = virXPathString("string(./product/@name)", ctxt);
> +
> + if (product != NULL) {
> + source->product = strdup(product);
> + }
> +
Back to the mutually-essential question - per your current .rng, you
should issue an error here if (product==NULL)!=(vendor==NULL). But is
that really essential, or does it make sense that you could have one and
not the other?
> ret = 0;
> cleanup:
> ctxt->node = relnode;
> @@ -838,6 +854,12 @@
> virBufferVSprintf(buf," <auth type='chap' login='%s'
> passwd='%s'/>\n",
> src->auth.chap.login,
> src->auth.chap.passwd);
> +
> + if (src->vendor != NULL && src->product != NULL) {
> + virBufferVSprintf(buf," <vendor name='%s'/>\n", src->vendor);
> + virBufferVSprintf(buf," <product name='%s'/>\n", src->product);
> + }
More code that assumes both elements will either be present or absent
together.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list