Re: [PATCH 17/17] virDomainTimerDefParseXML: Switch to virXMLPropEnumDefault()

[Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>]

On 5/23/22 3:08 PM, Michal Privoznik wrote:
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 52a34cd131..27fe6c9fbf 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11962,98 +11962,61 @@ virDomainTimerDefParseXML(xmlNodePtr node,
>       virDomainTimerDef *def;
>       VIR_XPATH_NODE_AUTORESTORE(ctxt)
>       xmlNodePtr catchup;
> -    int ret;
> -    g_autofree char *name = NULL;
> -    g_autofree char *tickpolicy = NULL;
> -    g_autofree char *track = NULL;
> -    g_autofree char *mode = NULL;
>   
>       def = g_new0(virDomainTimerDef, 1);
>   
>       ctxt->node = node;
>   
> -    name = virXMLPropString(node, "name");
> -    if (name == NULL) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       "%s", _("missing timer name"));
> +    if (virXMLPropEnum(node, "name",
> +                       virDomainTimerNameTypeFromString,
> +                       VIR_XML_PROP_REQUIRED,
> +                       &def->name) < 0)
>           goto error;
> -    }
> -    if ((def->name = virDomainTimerNameTypeFromString(name)) < 0) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       _("unknown timer name '%s'"), name);
> -        goto error;
> -    }
>   
>       if (virXMLPropTristateBool(node, "present",
>                                  VIR_XML_PROP_NONE,
>                                  &def->present) < 0)
>           goto error;
>   
> -    tickpolicy = virXMLPropString(node, "tickpolicy");
> -    if (tickpolicy != NULL) {
> -        if ((def->tickpolicy = virDomainTimerTickpolicyTypeFromString(tickpolicy)) <= 0) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                           _("unknown timer tickpolicy '%s'"), tickpolicy);
> -            goto error;
> -        }
> -    }
> +    if (virXMLPropEnum(node, "tickpolicy",
> +                       virDomainTimerTickpolicyTypeFromString,
> +                       VIR_XML_PROP_NONZERO,
> +                       &def->tickpolicy) < 0)
> +        goto error;
>   
> -    track = virXMLPropString(node, "track");
> -    if (track != NULL) {
> -        if ((def->track = virDomainTimerTrackTypeFromString(track)) <= 0) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                           _("unknown timer track '%s'"), track);
> -            goto error;
> -        }
> -    }
> +    if (virXMLPropEnum(node, "track",
> +                       virDomainTimerTrackTypeFromString,
> +                       VIR_XML_PROP_NONZERO,
> +                       &def->track) < 0)
> +        goto error;
>   
> -    ret = virXPathULongLong("string(./@frequency)", ctxt, &def->frequency);
> -    if (ret == -1) {
> -        def->frequency = 0;

Is the above case covered in virXMLPropULongLong?
A few other cases like following below.

> -    } else if (ret < 0) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       "%s", _("invalid timer frequency"));
> +    if (virXMLPropULongLong(node, "frequency", 10,
> +                            VIR_XML_PROP_NONE,
> +                            &def->frequency) < 0)
>           goto error;
> -    }
>   
> -    mode = virXMLPropString(node, "mode");
> -    if (mode != NULL) {
> -        if ((def->mode = virDomainTimerModeTypeFromString(mode)) <= 0) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                           _("unknown timer mode '%s'"), mode);
> -            goto error;
> -        }
> -    }
> +    if (virXMLPropEnum(node, "mode",
> +                       virDomainTimerModeTypeFromString,
> +                       VIR_XML_PROP_NONZERO,
> +                       &def->mode) < 0)
> +        goto error;
>   
>       catchup = virXPathNode("./catchup", ctxt);
>       if (catchup != NULL) {
> -        ret = virXPathULong("string(./catchup/@threshold)", ctxt,
> -                            &def->catchup.threshold);
> -        if (ret == -1) {
> -            def->catchup.threshold = 0;
> -        } else if (ret < 0) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           "%s", _("invalid catchup threshold"));
> +        if (virXMLPropUInt(catchup, "threshold", 10,
> +                           VIR_XML_PROP_NONE,
> +                           &def->catchup.threshold) < 0)
>               goto error;
> -        }
>   
> -        ret = virXPathULong("string(./catchup/@slew)", ctxt, &def->catchup.slew);
> -        if (ret == -1) {
> -            def->catchup.slew = 0;
> -        } else if (ret < 0) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           "%s", _("invalid catchup slew"));
> +        if (virXMLPropUInt(catchup, "slew", 10,
> +                           VIR_XML_PROP_NONE,
> +                           &def->catchup.slew) < 0)
>               goto error;
> -        }
>   
> -        ret = virXPathULong("string(./catchup/@limit)", ctxt, &def->catchup.limit);
> -        if (ret == -1) {
> -            def->catchup.limit = 0;
> -        } else if (ret < 0) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           "%s", _("invalid catchup limit"));
> +        if (virXMLPropUInt(catchup, "limit", 10,
> +                           VIR_XML_PROP_NONE,
> +                           &def->catchup.limit) < 0)
>               goto error;
> -        }
>       }
>   
>       return def;
> @@ -26197,11 +26160,11 @@ virDomainTimerDefFormat(virBuffer *buf,
>       }
>   
>       if (def->catchup.threshold > 0)
> -        virBufferAsprintf(&catchupAttr, " threshold='%lu'", def->catchup.threshold);
> +        virBufferAsprintf(&catchupAttr, " threshold='%u'", def->catchup.threshold);
>       if (def->catchup.slew > 0)
> -        virBufferAsprintf(&catchupAttr, " slew='%lu'", def->catchup.slew);
> +        virBufferAsprintf(&catchupAttr, " slew='%u'", def->catchup.slew);
>       if (def->catchup.limit > 0)
> -        virBufferAsprintf(&catchupAttr, " limit='%lu'", def->catchup.limit);
> +        virBufferAsprintf(&catchupAttr, " limit='%u'", def->catchup.limit);
>   
>       virXMLFormatElement(&timerChld, "catchup", &catchupAttr, NULL);
>       virXMLFormatElement(buf, "timer", &timerAttr, &timerChld);


--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294