conf: report errors when parsing video resolution

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.

Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5c8672925899bfa30e2c71d55b6053871b3a7cc8..b0599c7318dd172f99358e4c0f989f55dc2e3c7b 100644 (file)
--- 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;
                 }