On Mon, Oct 24, 2011 at 12:07:29PM -0400, Stefan Berger wrote:
> This patch modifies the NWFilter parameter parser to support multiple
> elements with the same name and to internally build a list of items.
> An example of the XML looks like this:
>
> <parameter name='TEST' value='10.1.2.3'/>
> <parameter name='TEST' value='10.2.3.4'/>
> <parameter name='TEST' value='10.1.1.1'/>
>
> The list of values is then stored in the newly introduced data type
> virNWFilterVarValue.
>
> The XML formatter is also adapted to print out all items in alphabetical
> order sorted by 'name'.
>
> This patch als fixes a bug in the XML schema on the way.
>
> v3:
> - Follow Daniel Berrange's suggestion of parsing a list as shown above
> - Rewrote XML formatter to print out parameters in alphabetical order
>
> v2:
> - check that each item in the list only contains allowed characters
>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
>
> ---
> docs/schemas/nwfilter.rng | 18 +++++----
> src/conf/nwfilter_params.c | 86 ++++++++++++++++++++++++++++-----------------
> 2 files changed, 64 insertions(+), 40 deletions(-)
>
> Index: libvirt-acl/src/conf/nwfilter_params.c
> ===================================================================
> --- libvirt-acl.orig/src/conf/nwfilter_params.c
> +++ libvirt-acl/src/conf/nwfilter_params.c
> @@ -595,7 +595,7 @@ isValidVarName(const char *var)
> static bool
> isValidVarValue(const char *value)
> {
> - return value[strspn(value, VALID_VARVALUE)] == 0;
> + return (value[strspn(value, VALID_VARVALUE)] == 0) && (strlen(value) != 0);
> }
>
> static virNWFilterVarValuePtr
> @@ -627,15 +627,22 @@ virNWFilterParseParamAttributes(xmlNodeP
> if (nam != NULL && val != NULL) {
> if (!isValidVarName(nam))
> goto skip_entry;
> - value = virNWFilterParseVarValue(val);
> - if (!value)
> + if (!isValidVarValue(val))
> goto skip_entry;
> - if (virNWFilterHashTablePut(table, nam, value, 1)) {
> - VIR_FREE(nam);
> - VIR_FREE(val);
> - virNWFilterVarValueFree(value);
> - virNWFilterHashTableFree(table);
> - return NULL;
> + value = virHashLookup(table->hashTable, nam);
> + if (value) {
> + /* add value to existing value -> list */
> + if (!virNWFilterVarValueAddValue(value, val, false)) {
> + value = NULL;
> + goto err_exit;
> + }
> + val = NULL;
> + } else {
> + value = virNWFilterParseVarValue(val);
> + if (!value)
> + goto skip_entry;
> + if (virNWFilterHashTablePut(table, nam, value, 1))
> + goto err_exit;
> }
> value = NULL;
> }
> @@ -648,40 +655,55 @@ skip_entry:
> cur = cur->next;
> }
> return table;
> -}
> -
> -
> -struct formatterParam {
> - virBufferPtr buf;
> - const char *indent;
> -};
>
> +err_exit:
> + VIR_FREE(nam);
> + VIR_FREE(val);
> + virNWFilterVarValueFree(value);
> + virNWFilterHashTableFree(table);
> + return NULL;
> +}
>
> -static void
> -_formatParameterAttrs(void *payload, const void *name, void *data)
> +static int
> +virNWFilterFormatParameterNameSorter(const virHashKeyValuePairPtr a,
> + const virHashKeyValuePairPtr b)
> {
> - struct formatterParam *fp = (struct formatterParam *)data;
> - virNWFilterVarValuePtr value = payload;
> -
> - virBufferAsprintf(fp->buf, "%s<parameter name='%s' value='",
> - fp->indent,
> - (const char *)name);
> - virNWFilterVarValuePrint(value, fp->buf);
> - virBufferAddLit(fp->buf, "'/>\n");
> + return strcmp((const char *)a->key, (const char *)b->key);
> }
>
> -
> char *
> virNWFilterFormatParamAttributes(virNWFilterHashTablePtr table,
> const char *indent)
> {
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> - struct formatterParam fp = {
> - .buf = &buf,
> - .indent = indent,
> - };
> + char **keys, *key;
> + int i, j, card, numKeys;
> + virNWFilterVarValuePtr value;
> +
> + if (!table)
> + return NULL;
> +
> + keys = (char **)virHashGetKeys(table->hashTable,
> + virNWFilterFormatParameterNameSorter);
> + if (!keys)
> + return NULL;
> +
> + numKeys = virHashSize(table->hashTable);
> +
> + for (i = 0; i < numKeys; i++) {
> + value = virHashLookup(table->hashTable, keys[i]);
> + card = virNWFilterVarValueGetCardinality(value);
> +
> + for (j = 0; j < card; j++) {
> + virBufferAsprintf(&buf,
> + "%s<parameter name='%s' value='%s'/>\n",
> + indent, keys[i],
> + virNWFilterVarValueGetNthValue(value, j));
> + }
> + key = keys[i];
> + }
>
> - virHashForEach(table->hashTable, _formatParameterAttrs, &fp);
> + virHashFreeKeys(table->hashTable, (void **)keys);
>
> if (virBufferError(&buf)) {
> virReportOOMError();
> Index: libvirt-acl/docs/schemas/nwfilter.rng
> ===================================================================
> --- libvirt-acl.orig/docs/schemas/nwfilter.rng
> +++ libvirt-acl/docs/schemas/nwfilter.rng
> @@ -313,14 +313,16 @@
> <data type="NCName"/>
> </attribute>
> <optional>
> - <element name="parameter">
> - <attribute name="name">
> - <ref name="filter-param-name"/>
> - </attribute>
> - <attribute name="value">
> - <ref name="filter-param-value"/>
> - </attribute>
> - </element>
> + <zeroOrMore>
> + <element name="parameter">
> + <attribute name="name">
> + <ref name="filter-param-name"/>
> + </attribute>
> + <attribute name="value">
> + <ref name="filter-param-value"/>
> + </attribute>
> + </element>
> + </zeroOrMore>
> </optional>
> </define>
Since you have <zeroOrMore> I think you can remove the parent <optional>
element now
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list