Re: [libvirt] [PATCH 05/18] domain_conf: split graphics xml parser into multiple functions

[Ján Tomko]

On Mon, Apr 04, 2016 at 03:20:22PM +0200, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina 
> ---
>  src/conf/domain_conf.c | 835 
> +++--
>  1 file changed, 453 insertions(+), 382 deletions(-)
> 

s/ParseXMLVnc/ParseXMLVNC/g
s/ParseXMLSdl/ParseXMLSDL/g
s/ParseXMLRdp/ParseXMLRDP/g

would IMO look nicer.

> +
> +if ((def->type = virDomainGraphicsTypeFromString(type)) < 0) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("unknown graphics device type '%s'"), type);
> +goto error;
> +}
> +
> +if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0)
> +goto error;
> +
> +switch (def->type) {

You could also cast the type to the enum, to annoy anyone adding a new
graphics type to add it to this switch too.

ACK

Jan

> +case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
> +if (virDomainGraphicsDefParseXMLVnc(def, node, flags) < 0)
> +goto error;
> +break;
> +case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
> +if (virDomainGraphicsDefParseXMLSdl(def, node) < 0)
> +goto error;
> +break;
> +case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
> +if (virDomainGraphicsDefParseXMLRdp(def, node, flags) < 0)
> +goto error;
> +break;
> +case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
> +if (virDomainGraphicsDefParseXMLDesktop(def, node) < 0)
> +goto error;
> +break;
> +case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
> +if (virDomainGraphicsDefParseXMLSpice(def, node, flags) < 0)
> +goto error;
> +break;
>  }
>  

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

[libvirt] [PATCH 05/18] domain_conf: split graphics xml parser into multiple functions

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 src/conf/domain_conf.c | 835 +++--
 1 file changed, 453 insertions(+), 382 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9d48d07..fd9316f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10753,480 +10753,551 @@ 
virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def,
 }
 
 
-/* Parse the XML definition for a graphics device */
-static virDomainGraphicsDefPtr
-virDomainGraphicsDefParseXML(xmlNodePtr node,
- xmlXPathContextPtr ctxt,
- unsigned int flags)
+static int
+virDomainGraphicsDefParseXMLVnc(virDomainGraphicsDefPtr def,
+xmlNodePtr node,
+unsigned int flags)
 {
-virDomainGraphicsDefPtr def;
-char *type = NULL;
-
-if (VIR_ALLOC(def) < 0)
-return NULL;
-
-type = virXMLPropString(node, "type");
-if (!type) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("missing graphics device type"));
-goto error;
-}
-
-if ((def->type = virDomainGraphicsTypeFromString(type)) < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown graphics device type '%s'"), type);
-goto error;
-}
-
-if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0)
-goto error;
-
-if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
-char *port = virXMLPropString(node, "port");
-char *websocket = virXMLPropString(node, "websocket");
-char *sharePolicy = virXMLPropString(node, "sharePolicy");
-char *autoport;
+char *port = virXMLPropString(node, "port");
+char *websocket = virXMLPropString(node, "websocket");
+char *sharePolicy = virXMLPropString(node, "sharePolicy");
+char *autoport;
+int ret = -1;
 
-if (port) {
-if (virStrToLong_i(port, NULL, 10, >data.vnc.port) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("cannot parse vnc port %s"), port);
-VIR_FREE(port);
-goto error;
-}
+if (port) {
+if (virStrToLong_i(port, NULL, 10, >data.vnc.port) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse vnc port %s"), port);
 VIR_FREE(port);
-/* Legacy compat syntax, used -1 for auto-port */
-if (def->data.vnc.port == -1) {
-if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)
-def->data.vnc.port = 0;
-def->data.vnc.autoport = true;
-}
-} else {
-def->data.vnc.port = 0;
+goto error;
+}
+VIR_FREE(port);
+/* Legacy compat syntax, used -1 for auto-port */
+if (def->data.vnc.port == -1) {
+if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)
+def->data.vnc.port = 0;
 def->data.vnc.autoport = true;
 }
+} else {
+def->data.vnc.port = 0;
+def->data.vnc.autoport = true;
+}
 
-if ((autoport = virXMLPropString(node, "autoport")) != NULL) {
-if (STREQ(autoport, "yes")) {
-if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)
-def->data.vnc.port = 0;
-def->data.vnc.autoport = true;
-}
-VIR_FREE(autoport);
+if ((autoport = virXMLPropString(node, "autoport")) != NULL) {
+if (STREQ(autoport, "yes")) {
+if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)
+def->data.vnc.port = 0;
+def->data.vnc.autoport = true;
 }
+VIR_FREE(autoport);
+}
 
-if (websocket) {
-if (virStrToLong_i(websocket,
-   NULL, 10,
-   >data.vnc.websocket) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("cannot parse vnc WebSocket port %s"), 
websocket);
-VIR_FREE(websocket);
-goto error;
-}
+if (websocket) {
+if (virStrToLong_i(websocket,
+   NULL, 10,
+   >data.vnc.websocket) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse vnc WebSocket port %s"), websocket);
 VIR_FREE(websocket);
+goto error;
 }
+VIR_FREE(websocket);
+}
 
-if (sharePolicy) {
-int policy =
-   virDomainGraphicsVNCSharePolicyTypeFromString(sharePolicy);
+if (sharePolicy) {
+int policy =
+   virDomainGraphicsVNCSharePolicyTypeFromString(sharePolicy);
 
-if (policy < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-