[libvirt PATCH] All pointers to virXMLPropString() use g_autofree.
All modified functions are similar, in all cases "cleanup" label is removed, along with all the "goto" calls. Signed-off-by: Nicolas Brignone <nmbrignone@gmail.com> --- src/conf/device_conf.c | 183 +++++++++++++---------------------------- 1 file changed, 56 insertions(+), 127 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 7d48a3f..9fa6141 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -208,45 +208,43 @@ int virPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddressPtr addr) { - char *domain, *slot, *bus, *function, *multi; xmlNodePtr cur; xmlNodePtr zpci = NULL; - int ret = -1; memset(addr, 0, sizeof(*addr)); - domain = virXMLPropString(node, "domain"); - bus = virXMLPropString(node, "bus"); - slot = virXMLPropString(node, "slot"); - function = virXMLPropString(node, "function"); - multi = virXMLPropString(node, "multifunction"); + g_autofree char *domain = virXMLPropString(node, "domain"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *slot = virXMLPropString(node, "slot"); + g_autofree char *function = virXMLPropString(node, "function"); + g_autofree char *multi = virXMLPropString(node, "multifunction"); if (domain && virStrToLong_uip(domain, NULL, 0, &addr->domain) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'domain' attribute")); - goto cleanup; + return -1; } if (bus && virStrToLong_uip(bus, NULL, 0, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; } if (slot && virStrToLong_uip(slot, NULL, 0, &addr->slot) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'slot' attribute")); - goto cleanup; + return -1; } if (function && virStrToLong_uip(function, NULL, 0, &addr->function) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'function' attribute")); - goto cleanup; + return -1; } if (multi && @@ -254,11 +252,11 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown value '%s' for <address> 'multifunction' attribute"), multi); - goto cleanup; + return -1; } if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr, true)) - goto cleanup; + return -1; cur = node->children; while (cur) { @@ -270,17 +268,9 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, } if (zpci && virZPCIDeviceAddressParseXML(zpci, addr) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - VIR_FREE(domain); - VIR_FREE(bus); - VIR_FREE(slot); - VIR_FREE(function); - VIR_FREE(multi); - return ret; + return 0; } void @@ -309,187 +299,149 @@ int virDomainDeviceCCWAddressParseXML(xmlNodePtr node, virDomainDeviceCCWAddressPtr addr) { - int ret = -1; - char *cssid; - char *ssid; - char *devno; - memset(addr, 0, sizeof(*addr)); - cssid = virXMLPropString(node, "cssid"); - ssid = virXMLPropString(node, "ssid"); - devno = virXMLPropString(node, "devno"); + g_autofree char *cssid = virXMLPropString(node, "cssid"); + g_autofree char *ssid = virXMLPropString(node, "ssid"); + g_autofree char *devno = virXMLPropString(node, "devno"); if (cssid && ssid && devno) { if (cssid && virStrToLong_uip(cssid, NULL, 0, &addr->cssid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'cssid' attribute")); - goto cleanup; + return -1; } if (ssid && virStrToLong_uip(ssid, NULL, 0, &addr->ssid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'ssid' attribute")); - goto cleanup; + return -1; } if (devno && virStrToLong_uip(devno, NULL, 0, &addr->devno) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'devno' attribute")); - goto cleanup; + return -1; } if (!virDomainDeviceCCWAddressIsValid(addr)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid specification for virtio ccw" " address: cssid='%s' ssid='%s' devno='%s'"), cssid, ssid, devno); - goto cleanup; + return -1; } addr->assigned = true; } else if (cssid || ssid || devno) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid partial specification for virtio ccw" " address")); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(cssid); - VIR_FREE(ssid); - VIR_FREE(devno); - return ret; + return 0; } int virDomainDeviceDriveAddressParseXML(xmlNodePtr node, virDomainDeviceDriveAddressPtr addr) { - char *bus, *unit, *controller, *target; - int ret = -1; - memset(addr, 0, sizeof(*addr)); - controller = virXMLPropString(node, "controller"); - bus = virXMLPropString(node, "bus"); - target = virXMLPropString(node, "target"); - unit = virXMLPropString(node, "unit"); + g_autofree char *controller = virXMLPropString(node, "controller"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *target = virXMLPropString(node, "target"); + g_autofree char *unit = virXMLPropString(node, "unit"); if (controller && virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'controller' attribute")); - goto cleanup; + return -1; } if (bus && virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; } if (target && virStrToLong_uip(target, NULL, 10, &addr->target) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'target' attribute")); - goto cleanup; + return -1; } if (unit && virStrToLong_uip(unit, NULL, 10, &addr->unit) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'unit' attribute")); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(controller); - VIR_FREE(bus); - VIR_FREE(target); - VIR_FREE(unit); - return ret; + return 0; } int virDomainDeviceVirtioSerialAddressParseXML(xmlNodePtr node, virDomainDeviceVirtioSerialAddressPtr addr) { - char *controller, *bus, *port; - int ret = -1; - memset(addr, 0, sizeof(*addr)); - controller = virXMLPropString(node, "controller"); - bus = virXMLPropString(node, "bus"); - port = virXMLPropString(node, "port"); + g_autofree char *controller = virXMLPropString(node, "controller"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *port = virXMLPropString(node, "port"); if (controller && virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'controller' attribute")); - goto cleanup; + return -1; } if (bus && virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; } if (port && virStrToLong_uip(port, NULL, 10, &addr->port) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'port' attribute")); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(controller); - VIR_FREE(bus); - VIR_FREE(port); - return ret; + return 0; } int virDomainDeviceCcidAddressParseXML(xmlNodePtr node, virDomainDeviceCcidAddressPtr addr) { - char *controller, *slot; - int ret = -1; - memset(addr, 0, sizeof(*addr)); - controller = virXMLPropString(node, "controller"); - slot = virXMLPropString(node, "slot"); + g_autofree char *controller = virXMLPropString(node, "controller"); + g_autofree char *slot = virXMLPropString(node, "slot"); if (controller && virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'controller' attribute")); - goto cleanup; + return -1; } if (slot && virStrToLong_uip(slot, NULL, 10, &addr->slot) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'slot' attribute")); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(controller); - VIR_FREE(slot); - return ret; + return 0; } static int @@ -519,57 +471,41 @@ int virDomainDeviceUSBAddressParseXML(xmlNodePtr node, virDomainDeviceUSBAddressPtr addr) { - char *port, *bus; - int ret = -1; memset(addr, 0, sizeof(*addr)); - port = virXMLPropString(node, "port"); - bus = virXMLPropString(node, "bus"); + g_autofree char *port = virXMLPropString(node, "port"); + g_autofree char *bus = virXMLPropString(node, "bus"); if (port && virDomainDeviceUSBAddressParsePort(addr, port) < 0) - goto cleanup; + return -1; if (bus && virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; } - - ret = 0; - - cleanup: - VIR_FREE(bus); - VIR_FREE(port); - return ret; + return 0; } int virDomainDeviceSpaprVioAddressParseXML(xmlNodePtr node, virDomainDeviceSpaprVioAddressPtr addr) { - char *reg; - int ret; - memset(addr, 0, sizeof(*addr)); - reg = virXMLPropString(node, "reg"); + g_autofree char *reg = virXMLPropString(node, "reg"); if (reg) { if (virStrToLong_ull(reg, NULL, 16, &addr->reg) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'reg' attribute")); - ret = -1; - goto cleanup; + return -1; } addr->has_reg = true; } - - ret = 0; - cleanup: - VIR_FREE(reg); - return ret; + return 0; } bool @@ -604,19 +540,17 @@ int virInterfaceLinkParseXML(xmlNodePtr node, virNetDevIfLinkPtr lnk) { - int ret = -1; - char *stateStr, *speedStr; int state; - stateStr = virXMLPropString(node, "state"); - speedStr = virXMLPropString(node, "speed"); + g_autofree char *stateStr = virXMLPropString(node, "state"); + g_autofree char *speedStr = virXMLPropString(node, "speed"); if (stateStr) { if ((state = virNetDevIfStateTypeFromString(stateStr)) < 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown link state: %s"), stateStr); - goto cleanup; + return -1; } lnk->state = state; } @@ -626,14 +560,9 @@ virInterfaceLinkParseXML(xmlNodePtr node, virReportError(VIR_ERR_XML_ERROR, _("Unable to parse link speed: %s"), speedStr); - goto cleanup; + return -1; } - - ret = 0; - cleanup: - VIR_FREE(stateStr); - VIR_FREE(speedStr); - return ret; + return 0; } int -- 2.25.2
On 7/8/20 4:19 PM, Nicolas Brignone wrote:
All pointers to virXMLPropString() use g_autofree.
I changed the summary line like this, to be more precise: conf: use g_autofree for all pointers to virXMLPropString() in device_conf.c
All modified functions are similar, in all cases "cleanup" label is removed, along with all the "goto" calls.
I've been advised in the recent past to put the g_autofree additions and corresponding removals of free functions into one patch, then do the removal of the extra labels (in favor of directly returning) in a separate patch to make it easier to hand-verify / review. Here's a couple recent examples: https://www.redhat.com/archives/libvir-list/2020-July/msg00317.html In your case the changes are few enough that I'm okay with it a single patch, except...
Signed-off-by: Nicolas Brignone <nmbrignone@gmail.com> --- src/conf/device_conf.c | 183 +++++++++++++---------------------------- 1 file changed, 56 insertions(+), 127 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 7d48a3f..9fa6141 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -208,45 +208,43 @@ int virPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddressPtr addr) { - char *domain, *slot, *bus, *function, *multi; xmlNodePtr cur; xmlNodePtr zpci = NULL; - int ret = -1;
memset(addr, 0, sizeof(*addr));
- domain = virXMLPropString(node, "domain"); - bus = virXMLPropString(node, "bus"); - slot = virXMLPropString(node, "slot"); - function = virXMLPropString(node, "function"); - multi = virXMLPropString(node, "multifunction"); + g_autofree char *domain = virXMLPropString(node, "domain"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *slot = virXMLPropString(node, "slot"); + g_autofree char *function = virXMLPropString(node, "function"); + g_autofree char *multi = virXMLPropString(node, "multifunction");
... you've modified it so that local variables are being declared *below* a line of executable code rather than at the top of the block. Although I don't see anything in the coding style document (https://libvirt.org/coding-style.html) about it, and it may just be leftover from a time when we supported a compiler that required all declarations to be at top of scope, I think pretty much all of libvirts code declares all local variables at the top of the scope. That's simple enough for me to just fixup before pushing, so Reviewed-by: Laine Stump <laine@redhat.com> Congratulations on your first libvirt patch! :-)
if (domain && virStrToLong_uip(domain, NULL, 0, &addr->domain) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'domain' attribute")); - goto cleanup; + return -1; }
if (bus && virStrToLong_uip(bus, NULL, 0, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; }
if (slot && virStrToLong_uip(slot, NULL, 0, &addr->slot) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'slot' attribute")); - goto cleanup; + return -1; }
if (function && virStrToLong_uip(function, NULL, 0, &addr->function) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'function' attribute")); - goto cleanup; + return -1; }
if (multi && @@ -254,11 +252,11 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown value '%s' for <address> 'multifunction' attribute"), multi); - goto cleanup; + return -1;
} if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr, true)) - goto cleanup; + return -1;
cur = node->children; while (cur) { @@ -270,17 +268,9 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, }
if (zpci && virZPCIDeviceAddressParseXML(zpci, addr) < 0) - goto cleanup; + return -1;
- ret = 0; - - cleanup: - VIR_FREE(domain); - VIR_FREE(bus); - VIR_FREE(slot); - VIR_FREE(function); - VIR_FREE(multi); - return ret; + return 0; }
void @@ -309,187 +299,149 @@ int virDomainDeviceCCWAddressParseXML(xmlNodePtr node, virDomainDeviceCCWAddressPtr addr) { - int ret = -1; - char *cssid; - char *ssid; - char *devno; - memset(addr, 0, sizeof(*addr));
- cssid = virXMLPropString(node, "cssid"); - ssid = virXMLPropString(node, "ssid"); - devno = virXMLPropString(node, "devno"); + g_autofree char *cssid = virXMLPropString(node, "cssid"); + g_autofree char *ssid = virXMLPropString(node, "ssid"); + g_autofree char *devno = virXMLPropString(node, "devno");
if (cssid && ssid && devno) { if (cssid && virStrToLong_uip(cssid, NULL, 0, &addr->cssid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'cssid' attribute")); - goto cleanup; + return -1; } if (ssid && virStrToLong_uip(ssid, NULL, 0, &addr->ssid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'ssid' attribute")); - goto cleanup; + return -1; } if (devno && virStrToLong_uip(devno, NULL, 0, &addr->devno) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'devno' attribute")); - goto cleanup; + return -1; } if (!virDomainDeviceCCWAddressIsValid(addr)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid specification for virtio ccw" " address: cssid='%s' ssid='%s' devno='%s'"), cssid, ssid, devno); - goto cleanup; + return -1; } addr->assigned = true; } else if (cssid || ssid || devno) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid partial specification for virtio ccw" " address")); - goto cleanup; + return -1; }
- ret = 0; - - cleanup: - VIR_FREE(cssid); - VIR_FREE(ssid); - VIR_FREE(devno); - return ret; + return 0; }
int virDomainDeviceDriveAddressParseXML(xmlNodePtr node, virDomainDeviceDriveAddressPtr addr) { - char *bus, *unit, *controller, *target; - int ret = -1; - memset(addr, 0, sizeof(*addr));
- controller = virXMLPropString(node, "controller"); - bus = virXMLPropString(node, "bus"); - target = virXMLPropString(node, "target"); - unit = virXMLPropString(node, "unit"); + g_autofree char *controller = virXMLPropString(node, "controller"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *target = virXMLPropString(node, "target"); + g_autofree char *unit = virXMLPropString(node, "unit");
if (controller && virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'controller' attribute")); - goto cleanup; + return -1; }
if (bus && virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; }
if (target && virStrToLong_uip(target, NULL, 10, &addr->target) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'target' attribute")); - goto cleanup; + return -1; }
if (unit && virStrToLong_uip(unit, NULL, 10, &addr->unit) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'unit' attribute")); - goto cleanup; + return -1; }
- ret = 0; - - cleanup: - VIR_FREE(controller); - VIR_FREE(bus); - VIR_FREE(target); - VIR_FREE(unit); - return ret; + return 0; }
int virDomainDeviceVirtioSerialAddressParseXML(xmlNodePtr node, virDomainDeviceVirtioSerialAddressPtr addr) { - char *controller, *bus, *port; - int ret = -1; - memset(addr, 0, sizeof(*addr));
- controller = virXMLPropString(node, "controller"); - bus = virXMLPropString(node, "bus"); - port = virXMLPropString(node, "port"); + g_autofree char *controller = virXMLPropString(node, "controller"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *port = virXMLPropString(node, "port");
if (controller && virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'controller' attribute")); - goto cleanup; + return -1; }
if (bus && virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; }
if (port && virStrToLong_uip(port, NULL, 10, &addr->port) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'port' attribute")); - goto cleanup; + return -1; }
- ret = 0; - - cleanup: - VIR_FREE(controller); - VIR_FREE(bus); - VIR_FREE(port); - return ret; + return 0; }
int virDomainDeviceCcidAddressParseXML(xmlNodePtr node, virDomainDeviceCcidAddressPtr addr) { - char *controller, *slot; - int ret = -1; - memset(addr, 0, sizeof(*addr));
- controller = virXMLPropString(node, "controller"); - slot = virXMLPropString(node, "slot"); + g_autofree char *controller = virXMLPropString(node, "controller"); + g_autofree char *slot = virXMLPropString(node, "slot");
if (controller && virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'controller' attribute")); - goto cleanup; + return -1; }
if (slot && virStrToLong_uip(slot, NULL, 10, &addr->slot) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'slot' attribute")); - goto cleanup; + return -1; }
- ret = 0; - - cleanup: - VIR_FREE(controller); - VIR_FREE(slot); - return ret; + return 0; }
static int @@ -519,57 +471,41 @@ int virDomainDeviceUSBAddressParseXML(xmlNodePtr node, virDomainDeviceUSBAddressPtr addr) { - char *port, *bus; - int ret = -1;
memset(addr, 0, sizeof(*addr));
- port = virXMLPropString(node, "port"); - bus = virXMLPropString(node, "bus"); + g_autofree char *port = virXMLPropString(node, "port"); + g_autofree char *bus = virXMLPropString(node, "bus");
if (port && virDomainDeviceUSBAddressParsePort(addr, port) < 0) - goto cleanup; + return -1;
if (bus && virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; } - - ret = 0; - - cleanup: - VIR_FREE(bus); - VIR_FREE(port); - return ret; + return 0; }
int virDomainDeviceSpaprVioAddressParseXML(xmlNodePtr node, virDomainDeviceSpaprVioAddressPtr addr) { - char *reg; - int ret; - memset(addr, 0, sizeof(*addr));
- reg = virXMLPropString(node, "reg"); + g_autofree char *reg = virXMLPropString(node, "reg"); if (reg) { if (virStrToLong_ull(reg, NULL, 16, &addr->reg) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'reg' attribute")); - ret = -1; - goto cleanup; + return -1; }
addr->has_reg = true; } - - ret = 0; - cleanup: - VIR_FREE(reg); - return ret; + return 0; }
bool @@ -604,19 +540,17 @@ int virInterfaceLinkParseXML(xmlNodePtr node, virNetDevIfLinkPtr lnk) { - int ret = -1; - char *stateStr, *speedStr; int state;
- stateStr = virXMLPropString(node, "state"); - speedStr = virXMLPropString(node, "speed"); + g_autofree char *stateStr = virXMLPropString(node, "state"); + g_autofree char *speedStr = virXMLPropString(node, "speed");
if (stateStr) { if ((state = virNetDevIfStateTypeFromString(stateStr)) < 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown link state: %s"), stateStr); - goto cleanup; + return -1; } lnk->state = state; } @@ -626,14 +560,9 @@ virInterfaceLinkParseXML(xmlNodePtr node, virReportError(VIR_ERR_XML_ERROR, _("Unable to parse link speed: %s"), speedStr); - goto cleanup; + return -1; } - - ret = 0; - cleanup: - VIR_FREE(stateStr); - VIR_FREE(speedStr); - return ret; + return 0; }
int
On Wed, Jul 08, 2020 at 11:15:27PM -0400, Laine Stump wrote:
On 7/8/20 4:19 PM, Nicolas Brignone wrote:
All pointers to virXMLPropString() use g_autofree.
I changed the summary line like this, to be more precise:
conf: use g_autofree for all pointers to virXMLPropString() in device_conf.c
All modified functions are similar, in all cases "cleanup" label is removed, along with all the "goto" calls.
I've been advised in the recent past to put the g_autofree additions and corresponding removals of free functions into one patch, then do the removal of the extra labels (in favor of directly returning) in a separate patch to make it easier to hand-verify / review. Here's a couple recent examples:
https://www.redhat.com/archives/libvir-list/2020-July/msg00317.html
In your case the changes are few enough that I'm okay with it a single patch, except...
Signed-off-by: Nicolas Brignone <nmbrignone@gmail.com> --- src/conf/device_conf.c | 183 +++++++++++++---------------------------- 1 file changed, 56 insertions(+), 127 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 7d48a3f..9fa6141 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -208,45 +208,43 @@ int virPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddressPtr addr) { - char *domain, *slot, *bus, *function, *multi; xmlNodePtr cur; xmlNodePtr zpci = NULL; - int ret = -1; memset(addr, 0, sizeof(*addr)); - domain = virXMLPropString(node, "domain"); - bus = virXMLPropString(node, "bus"); - slot = virXMLPropString(node, "slot"); - function = virXMLPropString(node, "function"); - multi = virXMLPropString(node, "multifunction"); + g_autofree char *domain = virXMLPropString(node, "domain"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *slot = virXMLPropString(node, "slot"); + g_autofree char *function = virXMLPropString(node, "function"); + g_autofree char *multi = virXMLPropString(node, "multifunction");
... you've modified it so that local variables are being declared *below* a line of executable code rather than at the top of the block. Although I don't see anything in the coding style document (https://libvirt.org/coding-style.html) about it, and it may just be leftover from a time when we supported a compiler that required all declarations to be at top of scope, I think pretty much all of libvirts code declares all local variables at the top of the scope.
We should really document it, and even enforce it at compile time. When you have variables in the middle of a function, and do a goto jump over them, they are in scope at the goto label target, but you may have jumped over their initialization. This makes it hard to rationalize about correctness of the code and has led to bugs involving uninitialized data before. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Jul 9, 2020 at 5:15 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Wed, Jul 08, 2020 at 11:15:27PM -0400, Laine Stump wrote:
On 7/8/20 4:19 PM, Nicolas Brignone wrote:
All pointers to virXMLPropString() use g_autofree.
I changed the summary line like this, to be more precise:
conf: use g_autofree for all pointers to virXMLPropString() in device_conf.c
All modified functions are similar, in all cases "cleanup" label is removed, along with all the "goto" calls.
I've been advised in the recent past to put the g_autofree additions and corresponding removals of free functions into one patch, then do the removal of the extra labels (in favor of directly returning) in a separate patch to make it easier to hand-verify / review. Here's a couple recent examples:
https://www.redhat.com/archives/libvir-list/2020-July/msg00317.html
In your case the changes are few enough that I'm okay with it a single patch, except...
Signed-off-by: Nicolas Brignone <nmbrignone@gmail.com> --- src/conf/device_conf.c | 183 +++++++++++++---------------------------- 1 file changed, 56 insertions(+), 127 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 7d48a3f..9fa6141 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -208,45 +208,43 @@ int virPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddressPtr addr) { - char *domain, *slot, *bus, *function, *multi; xmlNodePtr cur; xmlNodePtr zpci = NULL; - int ret = -1; memset(addr, 0, sizeof(*addr)); - domain = virXMLPropString(node, "domain"); - bus = virXMLPropString(node, "bus"); - slot = virXMLPropString(node, "slot"); - function = virXMLPropString(node, "function"); - multi = virXMLPropString(node, "multifunction"); + g_autofree char *domain = virXMLPropString(node, "domain"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *slot = virXMLPropString(node, "slot"); + g_autofree char *function = virXMLPropString(node, "function"); + g_autofree char *multi = virXMLPropString(node, "multifunction");
... you've modified it so that local variables are being declared *below* a line of executable code rather than at the top of the block. Although I don't see anything in the coding style document (https://libvirt.org/coding-style.html) about it, and it may just be leftover from a time when we supported a compiler that required all declarations to be at top of scope, I think pretty much all of libvirts code declares all local variables at the top of the scope.
We should really document it, and even enforce it at compile time.
I think you are asking for enabling: -Wdeclaration-after-statement (C and Objective-C only) Warn when a declaration is found after a statement in a block. This construct, known from C++, was introduced with ISO C99 and is by default allowed in GCC. It is not supported by ISO C90. Will take a look.
When you have variables in the middle of a function, and do a goto jump over them, they are in scope at the goto label target, but you may have jumped over their initialization. This makes it hard to rationalize about correctness of the code and has led to bugs involving uninitialized data before.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Jul 09, 2020 at 02:40:34PM -0300, Nicolás Brignone wrote:
On Thu, Jul 9, 2020 at 5:15 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Wed, Jul 08, 2020 at 11:15:27PM -0400, Laine Stump wrote:
On 7/8/20 4:19 PM, Nicolas Brignone wrote:
All pointers to virXMLPropString() use g_autofree.
I changed the summary line like this, to be more precise:
conf: use g_autofree for all pointers to virXMLPropString() in device_conf.c
All modified functions are similar, in all cases "cleanup" label is removed, along with all the "goto" calls.
I've been advised in the recent past to put the g_autofree additions and corresponding removals of free functions into one patch, then do the removal of the extra labels (in favor of directly returning) in a separate patch to make it easier to hand-verify / review. Here's a couple recent examples:
https://www.redhat.com/archives/libvir-list/2020-July/msg00317.html
In your case the changes are few enough that I'm okay with it a single patch, except...
Signed-off-by: Nicolas Brignone <nmbrignone@gmail.com> --- src/conf/device_conf.c | 183 +++++++++++++---------------------------- 1 file changed, 56 insertions(+), 127 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 7d48a3f..9fa6141 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -208,45 +208,43 @@ int virPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddressPtr addr) { - char *domain, *slot, *bus, *function, *multi; xmlNodePtr cur; xmlNodePtr zpci = NULL; - int ret = -1; memset(addr, 0, sizeof(*addr)); - domain = virXMLPropString(node, "domain"); - bus = virXMLPropString(node, "bus"); - slot = virXMLPropString(node, "slot"); - function = virXMLPropString(node, "function"); - multi = virXMLPropString(node, "multifunction"); + g_autofree char *domain = virXMLPropString(node, "domain"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *slot = virXMLPropString(node, "slot"); + g_autofree char *function = virXMLPropString(node, "function"); + g_autofree char *multi = virXMLPropString(node, "multifunction");
... you've modified it so that local variables are being declared *below* a line of executable code rather than at the top of the block. Although I don't see anything in the coding style document (https://libvirt.org/coding-style.html) about it, and it may just be leftover from a time when we supported a compiler that required all declarations to be at top of scope, I think pretty much all of libvirts code declares all local variables at the top of the scope.
We should really document it, and even enforce it at compile time.
I think you are asking for enabling:
-Wdeclaration-after-statement (C and Objective-C only) Warn when a declaration is found after a statement in a block. This construct, known from C++, was introduced with ISO C99 and is by default allowed in GCC. It is not supported by ISO C90.
Yes, but....
Will take a look.
..don't spend your time on enabling -Wdeclaration-after-statement unless you are strongly motivated to do a lot of tedious work. I took at stab at it about 12 months ago, but didn't complete it as there are a surprising amount of subtle edge cases to deal with, so I never got as far as sending a patch proposal. FWIW, WIP is here https://gitlab.com/berrange/libvirt/-/tree/c99-vars but this is based against ancient version, not current git master, so I doubt it will rebase cleanly at all Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Ack, Thanks! On Thu, Jul 9, 2020 at 2:48 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Thu, Jul 09, 2020 at 02:40:34PM -0300, Nicolás Brignone wrote:
On Thu, Jul 9, 2020 at 5:15 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Wed, Jul 08, 2020 at 11:15:27PM -0400, Laine Stump wrote:
On 7/8/20 4:19 PM, Nicolas Brignone wrote:
All pointers to virXMLPropString() use g_autofree.
I changed the summary line like this, to be more precise:
conf: use g_autofree for all pointers to virXMLPropString() in device_conf.c
All modified functions are similar, in all cases "cleanup" label is removed, along with all the "goto" calls.
I've been advised in the recent past to put the g_autofree additions and corresponding removals of free functions into one patch, then do the removal of the extra labels (in favor of directly returning) in a separate patch to make it easier to hand-verify / review. Here's a couple recent examples:
https://www.redhat.com/archives/libvir-list/2020-July/msg00317.html
In your case the changes are few enough that I'm okay with it a single patch, except...
Signed-off-by: Nicolas Brignone <nmbrignone@gmail.com> --- src/conf/device_conf.c | 183 +++++++++++++---------------------------- 1 file changed, 56 insertions(+), 127 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 7d48a3f..9fa6141 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -208,45 +208,43 @@ int virPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddressPtr addr) { - char *domain, *slot, *bus, *function, *multi; xmlNodePtr cur; xmlNodePtr zpci = NULL; - int ret = -1; memset(addr, 0, sizeof(*addr)); - domain = virXMLPropString(node, "domain"); - bus = virXMLPropString(node, "bus"); - slot = virXMLPropString(node, "slot"); - function = virXMLPropString(node, "function"); - multi = virXMLPropString(node, "multifunction"); + g_autofree char *domain = virXMLPropString(node, "domain"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *slot = virXMLPropString(node, "slot"); + g_autofree char *function = virXMLPropString(node, "function"); + g_autofree char *multi = virXMLPropString(node, "multifunction");
... you've modified it so that local variables are being declared *below* a line of executable code rather than at the top of the block. Although I don't see anything in the coding style document (https://libvirt.org/coding-style.html) about it, and it may just be leftover from a time when we supported a compiler that required all declarations to be at top of scope, I think pretty much all of libvirts code declares all local variables at the top of the scope.
We should really document it, and even enforce it at compile time.
I think you are asking for enabling:
-Wdeclaration-after-statement (C and Objective-C only) Warn when a declaration is found after a statement in a block. This construct, known from C++, was introduced with ISO C99 and is by default allowed in GCC. It is not supported by ISO C90.
Yes, but....
Will take a look.
..don't spend your time on enabling -Wdeclaration-after-statement unless you are strongly motivated to do a lot of tedious work. I took at stab at it about 12 months ago, but didn't complete it as there are a surprising amount of subtle edge cases to deal with, so I never got as far as sending a patch proposal.
FWIW, WIP is here
https://gitlab.com/berrange/libvirt/-/tree/c99-vars
but this is based against ancient version, not current git master, so I doubt it will rebase cleanly at all
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Jul 9, 2020 at 12:15 AM Laine Stump <laine@redhat.com> wrote:
On 7/8/20 4:19 PM, Nicolas Brignone wrote:
All pointers to virXMLPropString() use g_autofree.
I changed the summary line like this, to be more precise:
conf: use g_autofree for all pointers to virXMLPropString() in device_conf.c
All modified functions are similar, in all cases "cleanup" label is removed, along with all the "goto" calls.
I've been advised in the recent past to put the g_autofree additions and corresponding removals of free functions into one patch, then do the removal of the extra labels (in favor of directly returning) in a separate patch to make it easier to hand-verify / review. Here's a
Makes sense, absolutely!
couple recent examples:
https://www.redhat.com/archives/libvir-list/2020-July/msg00317.html
In your case the changes are few enough that I'm okay with it a single patch, except...
Signed-off-by: Nicolas Brignone <nmbrignone@gmail.com> --- src/conf/device_conf.c | 183 +++++++++++++---------------------------- 1 file changed, 56 insertions(+), 127 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 7d48a3f..9fa6141 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -208,45 +208,43 @@ int virPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddressPtr addr) { - char *domain, *slot, *bus, *function, *multi; xmlNodePtr cur; xmlNodePtr zpci = NULL; - int ret = -1;
memset(addr, 0, sizeof(*addr));
- domain = virXMLPropString(node, "domain"); - bus = virXMLPropString(node, "bus"); - slot = virXMLPropString(node, "slot"); - function = virXMLPropString(node, "function"); - multi = virXMLPropString(node, "multifunction"); + g_autofree char *domain = virXMLPropString(node, "domain"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *slot = virXMLPropString(node, "slot"); + g_autofree char *function = virXMLPropString(node, "function"); + g_autofree char *multi = virXMLPropString(node, "multifunction");
... you've modified it so that local variables are being declared *below* a line of executable code rather than at the top of the block. Although I don't see anything in the coding style document (https://libvirt.org/coding-style.html) about it, and it may just be leftover from a time when we supported a compiler that required all declarations to be at top of scope, I think pretty much all of libvirts code declares all local variables at the top of the scope.
That's simple enough for me to just fixup before pushing, so
Thanks for that!
Reviewed-by: Laine Stump <laine@redhat.com>
Congratulations on your first libvirt patch! :-)
Thanks to you for reviewing so quickly and for the precise feedback. Regards!
if (domain && virStrToLong_uip(domain, NULL, 0, &addr->domain) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'domain' attribute")); - goto cleanup; + return -1; }
if (bus && virStrToLong_uip(bus, NULL, 0, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; }
if (slot && virStrToLong_uip(slot, NULL, 0, &addr->slot) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'slot' attribute")); - goto cleanup; + return -1; }
if (function && virStrToLong_uip(function, NULL, 0, &addr->function) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'function' attribute")); - goto cleanup; + return -1; }
if (multi && @@ -254,11 +252,11 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown value '%s' for <address> 'multifunction' attribute"), multi); - goto cleanup; + return -1;
} if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr, true)) - goto cleanup; + return -1;
cur = node->children; while (cur) { @@ -270,17 +268,9 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, }
if (zpci && virZPCIDeviceAddressParseXML(zpci, addr) < 0) - goto cleanup; + return -1;
- ret = 0; - - cleanup: - VIR_FREE(domain); - VIR_FREE(bus); - VIR_FREE(slot); - VIR_FREE(function); - VIR_FREE(multi); - return ret; + return 0; }
void @@ -309,187 +299,149 @@ int virDomainDeviceCCWAddressParseXML(xmlNodePtr node, virDomainDeviceCCWAddressPtr addr) { - int ret = -1; - char *cssid; - char *ssid; - char *devno; - memset(addr, 0, sizeof(*addr));
- cssid = virXMLPropString(node, "cssid"); - ssid = virXMLPropString(node, "ssid"); - devno = virXMLPropString(node, "devno"); + g_autofree char *cssid = virXMLPropString(node, "cssid"); + g_autofree char *ssid = virXMLPropString(node, "ssid"); + g_autofree char *devno = virXMLPropString(node, "devno");
if (cssid && ssid && devno) { if (cssid && virStrToLong_uip(cssid, NULL, 0, &addr->cssid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'cssid' attribute")); - goto cleanup; + return -1; } if (ssid && virStrToLong_uip(ssid, NULL, 0, &addr->ssid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'ssid' attribute")); - goto cleanup; + return -1; } if (devno && virStrToLong_uip(devno, NULL, 0, &addr->devno) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'devno' attribute")); - goto cleanup; + return -1; } if (!virDomainDeviceCCWAddressIsValid(addr)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid specification for virtio ccw" " address: cssid='%s' ssid='%s' devno='%s'"), cssid, ssid, devno); - goto cleanup; + return -1; } addr->assigned = true; } else if (cssid || ssid || devno) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid partial specification for virtio ccw" " address")); - goto cleanup; + return -1; }
- ret = 0; - - cleanup: - VIR_FREE(cssid); - VIR_FREE(ssid); - VIR_FREE(devno); - return ret; + return 0; }
int virDomainDeviceDriveAddressParseXML(xmlNodePtr node, virDomainDeviceDriveAddressPtr addr) { - char *bus, *unit, *controller, *target; - int ret = -1; - memset(addr, 0, sizeof(*addr));
- controller = virXMLPropString(node, "controller"); - bus = virXMLPropString(node, "bus"); - target = virXMLPropString(node, "target"); - unit = virXMLPropString(node, "unit"); + g_autofree char *controller = virXMLPropString(node, "controller"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *target = virXMLPropString(node, "target"); + g_autofree char *unit = virXMLPropString(node, "unit");
if (controller && virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'controller' attribute")); - goto cleanup; + return -1; }
if (bus && virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; }
if (target && virStrToLong_uip(target, NULL, 10, &addr->target) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'target' attribute")); - goto cleanup; + return -1; }
if (unit && virStrToLong_uip(unit, NULL, 10, &addr->unit) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'unit' attribute")); - goto cleanup; + return -1; }
- ret = 0; - - cleanup: - VIR_FREE(controller); - VIR_FREE(bus); - VIR_FREE(target); - VIR_FREE(unit); - return ret; + return 0; }
int virDomainDeviceVirtioSerialAddressParseXML(xmlNodePtr node, virDomainDeviceVirtioSerialAddressPtr addr) { - char *controller, *bus, *port; - int ret = -1; - memset(addr, 0, sizeof(*addr));
- controller = virXMLPropString(node, "controller"); - bus = virXMLPropString(node, "bus"); - port = virXMLPropString(node, "port"); + g_autofree char *controller = virXMLPropString(node, "controller"); + g_autofree char *bus = virXMLPropString(node, "bus"); + g_autofree char *port = virXMLPropString(node, "port");
if (controller && virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'controller' attribute")); - goto cleanup; + return -1; }
if (bus && virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; }
if (port && virStrToLong_uip(port, NULL, 10, &addr->port) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'port' attribute")); - goto cleanup; + return -1; }
- ret = 0; - - cleanup: - VIR_FREE(controller); - VIR_FREE(bus); - VIR_FREE(port); - return ret; + return 0; }
int virDomainDeviceCcidAddressParseXML(xmlNodePtr node, virDomainDeviceCcidAddressPtr addr) { - char *controller, *slot; - int ret = -1; - memset(addr, 0, sizeof(*addr));
- controller = virXMLPropString(node, "controller"); - slot = virXMLPropString(node, "slot"); + g_autofree char *controller = virXMLPropString(node, "controller"); + g_autofree char *slot = virXMLPropString(node, "slot");
if (controller && virStrToLong_uip(controller, NULL, 10, &addr->controller) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'controller' attribute")); - goto cleanup; + return -1; }
if (slot && virStrToLong_uip(slot, NULL, 10, &addr->slot) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'slot' attribute")); - goto cleanup; + return -1; }
- ret = 0; - - cleanup: - VIR_FREE(controller); - VIR_FREE(slot); - return ret; + return 0; }
static int @@ -519,57 +471,41 @@ int virDomainDeviceUSBAddressParseXML(xmlNodePtr node, virDomainDeviceUSBAddressPtr addr) { - char *port, *bus; - int ret = -1;
memset(addr, 0, sizeof(*addr));
- port = virXMLPropString(node, "port"); - bus = virXMLPropString(node, "bus"); + g_autofree char *port = virXMLPropString(node, "port"); + g_autofree char *bus = virXMLPropString(node, "bus");
if (port && virDomainDeviceUSBAddressParsePort(addr, port) < 0) - goto cleanup; + return -1;
if (bus && virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'bus' attribute")); - goto cleanup; + return -1; } - - ret = 0; - - cleanup: - VIR_FREE(bus); - VIR_FREE(port); - return ret; + return 0; }
int virDomainDeviceSpaprVioAddressParseXML(xmlNodePtr node, virDomainDeviceSpaprVioAddressPtr addr) { - char *reg; - int ret; - memset(addr, 0, sizeof(*addr));
- reg = virXMLPropString(node, "reg"); + g_autofree char *reg = virXMLPropString(node, "reg"); if (reg) { if (virStrToLong_ull(reg, NULL, 16, &addr->reg) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot parse <address> 'reg' attribute")); - ret = -1; - goto cleanup; + return -1; }
addr->has_reg = true; } - - ret = 0; - cleanup: - VIR_FREE(reg); - return ret; + return 0; }
bool @@ -604,19 +540,17 @@ int virInterfaceLinkParseXML(xmlNodePtr node, virNetDevIfLinkPtr lnk) { - int ret = -1; - char *stateStr, *speedStr; int state;
- stateStr = virXMLPropString(node, "state"); - speedStr = virXMLPropString(node, "speed"); + g_autofree char *stateStr = virXMLPropString(node, "state"); + g_autofree char *speedStr = virXMLPropString(node, "speed");
if (stateStr) { if ((state = virNetDevIfStateTypeFromString(stateStr)) < 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown link state: %s"), stateStr); - goto cleanup; + return -1; } lnk->state = state; } @@ -626,14 +560,9 @@ virInterfaceLinkParseXML(xmlNodePtr node, virReportError(VIR_ERR_XML_ERROR, _("Unable to parse link speed: %s"), speedStr); - goto cleanup; + return -1; } - - ret = 0; - cleanup: - VIR_FREE(stateStr); - VIR_FREE(speedStr); - return ret; + return 0; }
int
participants (4)
-
Daniel P. Berrangé -
Laine Stump -
Nicolas Brignone -
Nicolás Brignone