[libvirt PATCH v4 1/8] virxml: Add virXMLPropTristateBool

[Peter Krempa]

On Thu, Apr 08, 2021 at 10:57:03 +0200, Tim Wiederhake wrote:
> Convenience function to return the value of a yes / no XML attribute.
> 
> Signed-off-by: Tim Wiederhake <twiederh at redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virxml.c        | 49 ++++++++++++++++++++++++++++++++++++++++
>  src/util/virxml.h        | 10 ++++++++
>  3 files changed, 60 insertions(+)

[...]

> diff --git a/src/util/virxml.c b/src/util/virxml.c
> index 4a6fe09468..0b822d7c4d 100644
> --- a/src/util/virxml.c
> +++ b/src/util/virxml.c
> @@ -558,6 +558,55 @@ virXMLNodeContentString(xmlNodePtr node)
>  }
>  
>  
> +/**
> + * virXMLPropTristateBool:
> + * @node: XML dom node pointer
> + * @name: Name of the property (attribute) to get
> + * @flags: Bitwise or of virXMLPropFlags
> + * @result: The returned value
> + *
> + * Convenience function to return value of a yes / no attribute.
> + *
> + * Returns 1 in case of success in which case @result is set,
> + *         or 0 if the attribute is not present,
> + *         or -1 and reports an error on failure.
> + */
> +int
> +virXMLPropTristateBool(xmlNodePtr node, const char* name,
> +                       virXMLPropFlags flags, virTristateBool *result)

One argument per line please.

> +{
> +    g_autofree char *tmp = NULL;
> +    int val;
> +
> +    if (!node || !name || !result) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Invalid argument to %s"),
> +                       __FUNCTION__);

Preferably declare the function with ATTRIBUTE_NONNULL instead of this
check. Additionally virReportError already records the function name at
least in the log message.

> +        return -1;
> +    }
> +
> +    if (!(tmp = virXMLPropString(node, name))) {
> +        if ((flags & VIR_XML_PROP_REQUIRED) != VIR_XML_PROP_REQUIRED)

The part after the masking (!= ... ) is redundant since you mask only
one bit out.

> +            return 0;
> +
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("Missing required attribute '%s' in element '%s'"),
> +                       name, node->name);
> +        return -1;
> +    }
> +
> +    if ((val = virTristateBoolTypeFromString(tmp)) <= 0) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("Invalid value for attribute '%s' in element '%s': '%s'. Expected 'yes' or 'no'"),
> +                       name, node->name, tmp);
> +        return -1;
> +    }
> +
> +    *result = val;
> +    return 1;
> +}
> +
> +
>  /**
>   * virXPathBoolean:
>   * @xpath: the XPath string to evaluate
> diff --git a/src/util/virxml.h b/src/util/virxml.h
> index d32f77b867..53de416a7f 100644
> --- a/src/util/virxml.h
> +++ b/src/util/virxml.h
> @@ -28,10 +28,16 @@
>  #include <libxml/relaxng.h>
>  
>  #include "virbuffer.h"
> +#include "virenum.h"
>  
>  xmlXPathContextPtr virXMLXPathContextNew(xmlDocPtr xml)
>      G_GNUC_WARN_UNUSED_RESULT;
>  
> +typedef enum {
> +    VIR_XML_PROP_OPTIONAL       = 0,        /* Attribute may be absent */
> +    VIR_XML_PROP_REQUIRED       = 1 << 0,   /* Attribute may not be absent */
> +} virXMLPropFlags;

Please avoid whitespace alignments.

> +
>  int              virXPathBoolean(const char *xpath,
>                                   xmlXPathContextPtr ctxt);
>  char *            virXPathString(const char *xpath,
> @@ -77,6 +83,10 @@ char *     virXMLPropStringLimit(xmlNodePtr node,
>                                   const char *name,
>                                   size_t maxlen);
>  char *   virXMLNodeContentString(xmlNodePtr node);
> +int       virXMLPropTristateBool(xmlNodePtr node,
> +                                 const char *name,
> +                                 virXMLPropFlags flags,
> +                                 virTristateBool *result);


Same here, please declare the function using the new formatting style,
or at least avoid the extraneous pointless whitespace.

>  
>  /* Internal function; prefer the macros below.  */
>  xmlDocPtr      virXMLParseHelper(int domcode,
> -- 
> 2.26.2
>