In virDomainVideoModelDefParseXML(), use the virXMLProp* functions
rather than reimplementing them with virXPath* functions.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
src/conf/domain_conf.c | 78 +++++++++++++-----------------------------
1 file changed, 23 insertions(+), 55 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2e153db94f..552936c8b7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12543,19 +12543,15 @@ virDomainVideoModelDefParseXML(virDomainVideoDef *def,
{
xmlNodePtr accel_node;
xmlNodePtr res_node;
- g_autofree char *type = NULL;
- g_autofree char *heads = NULL;
- g_autofree char *vram = NULL;
- g_autofree char *vram64 = NULL;
- g_autofree char *ram = NULL;
- g_autofree char *vgamem = NULL;
- g_autofree char *primary = NULL;
+ virDomainVideoType type;
+ virTristateBool primary;
+ int rc = 0;
VIR_XPATH_NODE_AUTORESTORE(ctxt);
ctxt->node = node;
- if ((primary = virXPathString("string(./@primary)", ctxt)) != NULL)
- ignore_value(virStringParseYesNo(primary, &def->primary));
+ if (virXMLPropTristateBool(node, "primary", VIR_XML_PROP_NONE, &primary) >= 0)
+ def->primary = (primary == VIR_TRISTATE_BOOL_YES);
if ((accel_node = virXPathNode("./acceleration", ctxt)) &&
(def->accel = virDomainVideoAccelDefParseXML(accel_node)) == NULL)
@@ -12565,57 +12561,29 @@ virDomainVideoModelDefParseXML(virDomainVideoDef *def,
(def->res = virDomainVideoResolutionDefParseXML(res_node)) == NULL)
return -1;
+ if (virXMLPropEnumDefault(node, "type",
+ virDomainVideoTypeFromString,
+ VIR_XML_PROP_NONE, &type,
+ VIR_DOMAIN_VIDEO_TYPE_DEFAULT) < 0)
+ return -1;
+ def->type = type;
+ if (virXMLPropUInt(node, "ram", 10, VIR_XML_PROP_NONE, &def->ram) < 0)
+ return -1;
- if ((type = virXPathString("string(./@type)", ctxt))) {
- if ((def->type = virDomainVideoTypeFromString(type)) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown video model '%s'"), type);
- return -1;
- }
- } else {
- def->type = VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
- }
-
- if ((ram = virXPathString("string(./@ram)", ctxt))) {
- if (virStrToLong_uip(ram, NULL, 10, &def->ram) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("cannot parse video ram '%s'"), ram);
- return -1;
- }
- }
-
- if ((vram = virXPathString("string(./@vram)", ctxt))) {
- if (virStrToLong_uip(vram, NULL, 10, &def->vram) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("cannot parse video vram '%s'"), vram);
- return -1;
- }
- }
+ if (virXMLPropUInt(node, "vram", 10, VIR_XML_PROP_NONE, &def->vram) < 0)
+ return -1;
- if ((vram64 = virXPathString("string(./@vram64)", ctxt))) {
- if (virStrToLong_uip(vram64, NULL, 10, &def->vram64) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("cannot parse video vram64 '%s'"), vram64);
- return -1;
- }
- }
+ if (virXMLPropUInt(node, "vram64", 10, VIR_XML_PROP_NONE, &def->vram64) < 0)
+ return -1;
- if ((vgamem = virXPathString("string(./@vgamem)", ctxt))) {
- if (virStrToLong_uip(vgamem, NULL, 10, &def->vgamem) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("cannot parse video vgamem '%s'"), vgamem);
- return -1;
- }
- }
+ if (virXMLPropUInt(node, "vgamem", 10, VIR_XML_PROP_NONE, &def->vgamem) < 0)
+ return -1;
- if ((heads = virXPathString("string(./@heads)", ctxt))) {
- if (virStrToLong_uip(heads, NULL, 10, &def->heads) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse video heads '%s'"), heads);
- return -1;
- }
- }
+ if ((rc = virXMLPropUInt(node, "heads", 10, VIR_XML_PROP_NONE, &def->heads)) < 0)
+ return -1;
+ else if (rc == 0)
+ def->heads = 1;
return 0;
}
--
2.38.1On a Friday in 2022, Jonathon Jongsma wrote:
>In virDomainVideoModelDefParseXML(), use the virXMLProp* functions
>rather than reimplementing them with virXPath* functions.
>
>Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
>---
> src/conf/domain_conf.c | 78 +++++++++++++-----------------------------
> 1 file changed, 23 insertions(+), 55 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 2e153db94f..552936c8b7 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>- if ((heads = virXPathString("string(./@heads)", ctxt))) {
>- if (virStrToLong_uip(heads, NULL, 10, &def->heads) < 0) {
>- virReportError(VIR_ERR_INTERNAL_ERROR,
>- _("cannot parse video heads '%s'"), heads);
>- return -1;
>- }
>- }
>+ if ((rc = virXMLPropUInt(node, "heads", 10, VIR_XML_PROP_NONE, &def->heads)) < 0)
>+ return -1;
>+ else if (rc == 0)
>+ def->heads = 1;
This branch is not necessary - just like the previous code, def->heads
is untouched if the attribute is not present.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
On 11/7/22 1:45 AM, Ján Tomko wrote:
> On a Friday in 2022, Jonathon Jongsma wrote:
>> In virDomainVideoModelDefParseXML(), use the virXMLProp* functions
>> rather than reimplementing them with virXPath* functions.
>>
>> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
>> ---
>> src/conf/domain_conf.c | 78 +++++++++++++-----------------------------
>> 1 file changed, 23 insertions(+), 55 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 2e153db94f..552936c8b7 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> - if ((heads = virXPathString("string(./@heads)", ctxt))) {
>> - if (virStrToLong_uip(heads, NULL, 10, &def->heads) < 0) {
>> - virReportError(VIR_ERR_INTERNAL_ERROR,
>> - _("cannot parse video heads '%s'"), heads);
>> - return -1;
>> - }
>> - }
>> + if ((rc = virXMLPropUInt(node, "heads", 10, VIR_XML_PROP_NONE,
>> &def->heads)) < 0)
>> + return -1;
>
>> + else if (rc == 0)
>> + def->heads = 1;
>
> This branch is not necessary - just like the previous code, def->heads
> is untouched if the attribute is not present.
This is unfortunately not true. virXMLPropUInt() actually sets *result =
0 at the very beginning of the function. So this branch is necessary
unless we change the implementation of virXMLPropUInt().
>
> Reviewed-by: Ján Tomko <jtomko@redhat.com>
>
> Jano
>
On Tue, Nov 08, 2022 at 12:04:49 -0600, Jonathon Jongsma wrote:
> On 11/7/22 1:45 AM, Ján Tomko wrote:
> > On a Friday in 2022, Jonathon Jongsma wrote:
> > > In virDomainVideoModelDefParseXML(), use the virXMLProp* functions
> > > rather than reimplementing them with virXPath* functions.
> > >
> > > Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> > > ---
> > > src/conf/domain_conf.c | 78 +++++++++++++-----------------------------
> > > 1 file changed, 23 insertions(+), 55 deletions(-)
> > >
> > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > > index 2e153db94f..552936c8b7 100644
> > > --- a/src/conf/domain_conf.c
> > > +++ b/src/conf/domain_conf.c
> > > - if ((heads = virXPathString("string(./@heads)", ctxt))) {
> > > - if (virStrToLong_uip(heads, NULL, 10, &def->heads) < 0) {
> > > - virReportError(VIR_ERR_INTERNAL_ERROR,
> > > - _("cannot parse video heads '%s'"), heads);
> > > - return -1;
> > > - }
> > > - }
> > > + if ((rc = virXMLPropUInt(node, "heads", 10, VIR_XML_PROP_NONE,
> > > &def->heads)) < 0)
> > > + return -1;
> >
> > > + else if (rc == 0)
> > > + def->heads = 1;
> >
> > This branch is not necessary - just like the previous code, def->heads
> > is untouched if the attribute is not present.
>
> This is unfortunately not true. virXMLPropUInt() actually sets *result = 0
> at the very beginning of the function. So this branch is necessary unless we
> change the implementation of virXMLPropUInt().
Changing the impl is out of question, but you can factor out the
implementation and provide a version which sets the default value to
something else than 0. We have these for signed integer parsing helpers
and for enums as virXMLPropEnumDefault.
© 2016 - 2026 Red Hat, Inc.