[libvirt] [PATCH v2 3/7] bandwidth: Add format parsing functions

[Jiri Denemark]

On Tue, Jul 12, 2011 at 13:57:09 +0200, Michal Privoznik wrote:
> These functions take on input decimal numbers optionally followed
> by unit. Units are exactly the same as 'tc' accepts.
> ---
>  src/conf/domain_conf.c   |    3 +
>  src/conf/domain_conf.h   |    1 +
>  src/conf/network_conf.c  |    5 +
>  src/conf/network_conf.h  |    1 +
>  src/libvirt_private.syms |    1 +
>  src/util/network.c       |  203 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/network.h       |    2 +
>  7 files changed, 216 insertions(+), 0 deletions(-)

This can be greatly simplified once we forbid using units in bandwidth
attributes.

...
> diff --git a/src/util/network.c b/src/util/network.c
> index eb16e0c..476ecde 100644
> --- a/src/util/network.c
> +++ b/src/util/network.c
> @@ -10,6 +10,7 @@
>  
>  #include <config.h>
>  #include <arpa/inet.h>
> +#include <math.h>

BTW, why did you need to include math.h? Getting rid of it is another good
reason for removing units.

>  
>  #include "memory.h"
>  #include "network.h"
> @@ -21,6 +22,9 @@
...
> +/**
> + * virBandwidthParseXML:

The name here doesn't match the function this is supposed to document.

> + * @node: XML node
> + * @def: where to store the parsed result
> + *
> + * Parse bandwidth XML and store into given pointer
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int
> +virBandwidthDefParseNode(xmlNodePtr node, virBandwidthPtr def)
> +{
> +    int ret = -1;
> +    xmlNodePtr cur = node->children;
> +    xmlNodePtr in = NULL, out = NULL;
> +
> +    if (!node || !def ||
> +        !xmlStrEqual(node->name, BAD_CAST "bandwidth"))
> +        return -1;
> +
> +    memset(def, 0, sizeof(virBandwidth));

Using sizeof(*def) is better.

> +    while (cur) {
> +        if (cur->type == XML_ELEMENT_NODE) {
> +            if (xmlStrEqual(cur->name, BAD_CAST "inbound")) {
> +                if (in) {
> +                    virBandwidthError(VIR_ERR_XML_DETAIL, "%s",
> +                                      _("Only one child <inbound> "
> +                                        "element allowed"));
> +                    goto cleanup;
> +                }
> +                in = cur;
> +            } else if (xmlStrEqual(cur->name, BAD_CAST "outbound")) {
> +                if (out) {
> +                    virBandwidthError(VIR_ERR_XML_DETAIL, "%s",
> +                                      _("Only one child <outbound> "
> +                                        "element allowed"));
> +                    goto cleanup;
> +                }
> +                out = cur;
> +            } else {
> +                virBandwidthError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                                  _("Unknown element %s"),
> +                                  cur->name);
> +                goto cleanup;

AFAIK we ignore unknown XML elements in other XML parsing code, shouldn't we
do the same here?

...

Jirka