Re: [PATCH v2 1/4] util: xml: add virXMLPropertyCount

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, May 15, 2016 at 15:11:26 -0400, Cole Robinson wrote:
> Returns an integer count of the number of XML properties an element
> has. Will be used in future patches.
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virxml.c        | 17 +++++++++++++++++
>  src/util/virxml.h        |  1 +
>  3 files changed, 19 insertions(+)

[...]

> diff --git a/src/util/virxml.c b/src/util/virxml.c
> index 489bad8..86c021c 100644
> --- a/src/util/virxml.c
> +++ b/src/util/virxml.c
> @@ -890,6 +890,23 @@ virXMLChildElementCount(xmlNodePtr node)
>      return ret;
>  }
>  
> +/* Returns the number of node's properties, or -1 on error.  */
> +long
> +virXMLPropertyCount(xmlNodePtr node)
> +{
> +    long ret = 0;
> +    xmlAttrPtr cur = NULL;
> +
> +    if (!node || node->type != XML_ELEMENT_NODE)
> +        return -1;
> +    cur = node->properties;
> +    while (cur) {
> +        ret++;
> +        cur = cur->next;
> +    }
> +    return ret;
> +}

Function like this looks rather fishy. Checking according to the count
of attributes is very likely to break. I'd rather see us checking
explicitly that certain attributes are present or not.

I think this will make the code more fragile in the long therm with
little gain in the short term.

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]