[PATCH v3 2/4] conf: report errors when parsing video resolution

[Jonathon Jongsma <jjongsma@xxxxxxxxxx>]

The current code doesn't properly handle errors when parsing a video
device's resolution.  We were returning a NULL structure for the case
where 'x' or 'y' were missing. But for the other error cases, we were
logging an error (virReportError()), but still returning an
under-specified structure. That under-specified structure was used by
the calling function rather than properly reporting an error.

This patch changes the parse function to return NULL on any parsing
error and changes the calling function to report an error when NULL is
returned.

Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
---
 src/conf/domain_conf.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5c86729258..b0599c7318 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15309,24 +15309,26 @@ virDomainVideoResolutionDefParseXML(xmlNodePtr node)
     x = virXMLPropString(node, "x");
     y = virXMLPropString(node, "y");
 
-    if (!x || !y)
+    if (!x || !y) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("missing values for resolution"));
         return NULL;
+    }
 
     def = g_new0(virDomainVideoResolutionDef, 1);
 
     if (virStrToLong_uip(x, NULL, 10, &def->x) < 0) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("cannot parse video x-resolution '%s'"), x);
-        goto cleanup;
+        return NULL;
     }
 
     if (virStrToLong_uip(y, NULL, 10, &def->y) < 0) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("cannot parse video y-resolution '%s'"), y);
-        goto cleanup;
+        return NULL;
     }
 
- cleanup:
     return g_steal_pointer(&def);
 }
 
@@ -15415,8 +15417,10 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
                             virXMLNodeNameEqual(child, "acceleration"))
                             def->accel = virDomainVideoAccelDefParseXML(child);
                         if (def->res == NULL &&
-                            virXMLNodeNameEqual(child, "resolution"))
-                            def->res = virDomainVideoResolutionDefParseXML(child);
+                            virXMLNodeNameEqual(child, "resolution")) {
+                            if ((def->res = virDomainVideoResolutionDefParseXML(child)) == NULL)
+                                goto error;
+                        }
                     }
                     child = child->next;
                 }
-- 
2.21.0

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