[libvirt] [PATCH V3 3/4] Extend NWFilter parameter parser to cope with lists of values

[Daniel P. Berrange]

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 at linux.vnet.ibm.com>
> 
> ---
>  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 :|