Re: [libvirt] [PATCH 06/18] domain_conf: cleanup error paths for graphics parser
On Mon, Apr 04, 2016 at 03:20:23PM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/conf/domain_conf.c | 100 ++++++++++++++++++++++---------------------------
1 file changed, 45 insertions(+), 55 deletions(-)
@@ -10846,17 +10845,19 @@
virDomainGraphicsDefParseXMLSdl(virDomainGraphicsDefPtr def,
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown fullscreen value '%s'"),
fullscreen);
- VIR_FREE(fullscreen);
- return -1;
+ goto error;
}
- VIR_FREE(fullscreen);
} else {
def->data.sdl.fullscreen = false;
}
+
def->data.sdl.xauth = virXMLPropString(node, "xauth");
def->data.sdl.display = virXMLPropString(node, "display");
- return 0;
+ ret = 0;
+ error:
If you're adding a new label, 'cleanup' is better for paths shared by
the success and the error paths.
+ VIR_FREE(fullscreen);
+ return ret;
}
@@ -10866,52 +10867,44 @@
virDomainGraphicsDefParseXMLRdp(virDomainGraphicsDefPtr def,
}
- if ((autoport = virXMLPropString(node, "autoport")) != NULL) {
- if (STREQ(autoport, "yes"))
- def->data.rdp.autoport = true;
-
- VIR_FREE(autoport);
- }
+ if (autoport && STREQ(autoport, "yes"))
+ def->data.rdp.autoport = true;
Could be STREQ_NULLABLE.
if (def->data.rdp.autoport && (flags &
VIR_DOMAIN_DEF_PARSE_INACTIVE))
def->data.rdp.port = 0;
- if ((replaceUser = virXMLPropString(node, "replaceUser")) != NULL) {
- if (STREQ(replaceUser, "yes"))
- def->data.rdp.replaceUser = true;
- VIR_FREE(replaceUser);
- }
+ if (replaceUser && STREQ(replaceUser, "yes"))
+ def->data.rdp.replaceUser = true;
- if ((multiUser = virXMLPropString(node, "multiUser")) != NULL) {
- if (STREQ(multiUser, "yes"))
- def->data.rdp.multiUser = true;
- VIR_FREE(multiUser);
- }
+ if (multiUser && STREQ(multiUser, "yes"))
+ def->data.rdp.multiUser = true;
ret = 0;
error:
+ VIR_FREE(port);
+ VIR_FREE(autoport);
+ VIR_FREE(replaceUser);
+ VIR_FREE(multiUser);
return ret;
}
@@ -10930,16 +10924,18 @@
virDomainGraphicsDefParseXMLDesktop(virDomainGraphicsDefPtr def,
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown fullscreen value '%s'"),
fullscreen);
- VIR_FREE(fullscreen);
- return -1;
+ goto error;
}
- VIR_FREE(fullscreen);
} else {
def->data.desktop.fullscreen = false;
}
def->data.desktop.display = virXMLPropString(node, "display");
- return 0;
+
+ ret = 0;
+ error:
s/error/cleanup/
+ VIR_FREE(fullscreen);
+ return ret;
}
ACK
Jan