It costs quite a few processor cycles to go through printf parsing just to determine that we only meant to append. * src/xen/xend_internal.c (xend_op_ext): Consolidate multiple printfs into one. * src/qemu/qemu_command.c (qemuBuildWatchdogDevStr) (qemuBuildUSBInputDevStr, qemuBuildSoundDevStr) (qemuBuildSoundCodecStr, qemuBuildVideoDevStr): Likewise. (qemuBuildCpuArgStr, qemuBuildCommandLine): Prefer virBufferAdd over virBufferVsprintf for trivial appends. * src/phyp/phyp_driver.c (phypExec, phypUUIDTable_Push) (phypUUIDTable_Pull): Likewise. * src/conf/nwfilter_conf.c (macProtocolIDFormatter) (arpOpcodeFormatter, formatIPProtocolID, printStringItems) (virNWFilterPrintStateMatchFlags, virNWIPAddressFormat) (virNWFilterDefFormat): Likewise. * src/security/virt-aa-helper.c (main): Likewise. * src/util/sexpr.c (sexpr2string): Likewise. * src/xenxs/xen_sxpr.c (xenFormatSxprChr): Likewise. * src/xenxs/xen_xm.c (xenFormatXMDisk): Likewise. --- src/conf/nwfilter_conf.c | 16 ++++++++-------- src/phyp/phyp_driver.c | 7 +++---- src/qemu/qemu_command.c | 30 +++++++++++------------------- src/security/virt-aa-helper.c | 2 +- src/util/sexpr.c | 4 ++-- src/xen/xend_internal.c | 6 ++---- src/xenxs/xen_sxpr.c | 2 +- src/xenxs/xen_xm.c | 4 ++-- 8 files changed, 30 insertions(+), 41 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 0732322..327fab3 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -488,7 +488,7 @@ macProtocolIDFormatter(virBufferPtr buf, if (intMapGetByInt(macProtoMap, nwf->p.ethHdrFilter.dataProtocolID.u.u16, &str)) { - virBufferVSprintf(buf, "%s", str); + virBufferAdd(buf, str, -1); } else { if (nwf->p.ethHdrFilter.dataProtocolID.datatype == DATATYPE_UINT16) asHex = false; @@ -591,7 +591,7 @@ arpOpcodeFormatter(virBufferPtr buf, if (intMapGetByInt(arpOpcodeMap, nwf->p.arpHdrFilter.dataOpcode.u.u16, &str)) { - virBufferVSprintf(buf, "%s", str); + virBufferAdd(buf, str, -1); } else { virBufferVSprintf(buf, "%d", nwf->p.arpHdrFilter.dataOpcode.u.u16); } @@ -653,7 +653,7 @@ formatIPProtocolID(virBufferPtr buf, if (intMapGetByInt(ipProtoMap, nwf->p.ipHdrFilter.ipHdr.dataProtocolID.u.u8, &str)) { - virBufferVSprintf(buf, "%s", str); + virBufferAdd(buf, str, -1); } else { if (nwf->p.ipHdrFilter.ipHdr.dataProtocolID.datatype == DATATYPE_UINT8) asHex = false; @@ -734,8 +734,8 @@ printStringItems(virBufferPtr buf, const struct int_map *int_map, for (i = 0; int_map[i].val; i++) { if (mask == int_map[i].attr) { if (c >= 1) - virBufferVSprintf(buf, "%s", sep); - virBufferVSprintf(buf, "%s", int_map[i].val); + virBufferAdd(buf, sep, -1); + virBufferAdd(buf, int_map[i].val, -1); c++; } } @@ -769,7 +769,7 @@ virNWFilterPrintStateMatchFlags(virBufferPtr buf, const char *prefix, if (!disp_none && (flags & RULE_FLAG_STATE_NONE)) return; - virBufferVSprintf(buf, "%s", prefix); + virBufferAdd(buf, prefix, -1); printStringItems(buf, stateMatchMap, flags, ","); } @@ -2699,7 +2699,7 @@ virNWIPAddressFormat(virBufferPtr buf, virSocketAddrPtr ipaddr) char *output = virSocketFormatAddr(ipaddr); if (output) { - virBufferVSprintf(buf, "%s", output); + virBufferAdd(buf, output, -1); VIR_FREE(output); } } @@ -2936,7 +2936,7 @@ virNWFilterDefFormat(virNWFilterDefPtr def) xml = virNWFilterEntryFormat(def->filterEntries[i]); if (!xml) goto err_exit; - virBufferVSprintf(&buf, "%s", xml); + virBufferAdd(&buf, xml, -1); VIR_FREE(xml); } diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 3862c9c..5742d95 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1,4 +1,3 @@ - /* * Copyright (C) 2010-2011 Red Hat, Inc. * Copyright IBM Corp. 2009 @@ -156,7 +155,7 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, rc = libssh2_channel_read(channel, buffer, buffer_size); if (rc > 0) { bytecount += rc; - virBufferVSprintf(&tex_ret, "%s", buffer); + virBufferAdd(&tex_ret, buffer, -1); } } while (rc > 0); @@ -494,7 +493,7 @@ phypUUIDTable_Push(virConnectPtr conn) char *remote_file = NULL; if (conn->uri->user != NULL) { - virBufferVSprintf(&username, "%s", conn->uri->user); + virBufferAdd(&username, conn->uri->user, -1); if (virBufferError(&username)) { virBufferFreeAndReset(&username); @@ -711,7 +710,7 @@ phypUUIDTable_Pull(virConnectPtr conn) char *remote_file = NULL; if (conn->uri->user != NULL) { - virBufferVSprintf(&username, "%s", conn->uri->user); + virBufferAdd(&username, conn->uri->user, -1); if (virBufferError(&username)) { virBufferFreeAndReset(&username); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71e5062..2205ed1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1798,8 +1798,7 @@ qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev, goto error; } - virBufferVSprintf(&buf, "%s", model); - virBufferVSprintf(&buf, ",id=%s", dev->info.alias); + virBufferVSprintf(&buf, "%s,id=%s", model, dev->info.alias); if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0) goto error; @@ -1845,10 +1844,9 @@ qemuBuildUSBInputDevStr(virDomainInputDefPtr dev) { virBuffer buf = VIR_BUFFER_INITIALIZER; - virBufferVSprintf(&buf, "%s", + virBufferVSprintf(&buf, "%s,id=%s", dev->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? - "usb-mouse" : "usb-tablet"); - virBufferVSprintf(&buf, ",id=%s", dev->info.alias); + "usb-mouse" : "usb-tablet", dev->info.alias); if (virBufferError(&buf)) { virReportOOMError(); @@ -1884,8 +1882,7 @@ qemuBuildSoundDevStr(virDomainSoundDefPtr sound, else if (STREQ(model, "ich6")) model = "intel-hda"; - virBufferVSprintf(&buf, "%s", model); - virBufferVSprintf(&buf, ",id=%s", sound->info.alias); + virBufferVSprintf(&buf, "%s,id=%s", model, sound->info.alias); if (qemuBuildDeviceAddressStr(&buf, &sound->info, qemuCaps) < 0) goto error; @@ -1908,10 +1905,8 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, virBuffer buf = VIR_BUFFER_INITIALIZER; int cad = 0; - virBufferVSprintf(&buf, "%s", codec); - virBufferVSprintf(&buf, ",id=%s-codec%d", sound->info.alias, cad); - virBufferVSprintf(&buf, ",bus=%s.0", sound->info.alias); - virBufferVSprintf(&buf, ",cad=%d", cad); + virBufferVSprintf(&buf, "%s,id=%s-codec%d,bus=%s.0,cad=%d", + codec, sound->info.alias, cad, sound->info.alias, cad); if (virBufferError(&buf)) { virReportOOMError(); @@ -1938,8 +1933,7 @@ qemuBuildVideoDevStr(virDomainVideoDefPtr video, goto error; } - virBufferVSprintf(&buf, "%s", model); - virBufferVSprintf(&buf, ",id=%s", video->info.alias); + virBufferVSprintf(&buf, "%s,id=%s", model, video->info.alias); if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { if (video->vram > (UINT_MAX / 1024)) { @@ -2572,7 +2566,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, goto cleanup; *hasHwVirt = hasSVM > 0 ? true : false; - virBufferVSprintf(&buf, "%s", guest->model); + virBufferAdd(&buf, guest->model, -1); for (i = 0; i < guest->nfeatures; i++) { char sign; if (guest->features[i].policy == VIR_CPU_FEATURE_DISABLE) @@ -3128,7 +3122,7 @@ qemuBuildCommandLine(virConnectPtr conn, else if (def->os.bootmenu == VIR_DOMAIN_BOOT_MENU_DISABLED) virBufferVSprintf(&boot_buf, "order=%s,menu=off", boot); } else { - virBufferVSprintf(&boot_buf, "%s", boot); + virBufferAdd(&boot_buf, boot, -1); } virCommandAddArgBuffer(cmd, &boot_buf); @@ -3291,8 +3285,7 @@ qemuBuildCommandLine(virConnectPtr conn, host->name, host->port); } else { - virBufferVSprintf(&rbd_hosts, "%s", - host->name); + virBufferAdd(&rbd_hosts, host->name, -1); } } } @@ -3414,8 +3407,7 @@ qemuBuildCommandLine(virConnectPtr conn, host->name, host->port); } else { - virBufferVSprintf(&rbd_hosts, "%s", - host->name); + virBufferAdd(&rbd_hosts, host->name, -1); } } break; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index bb716e6..08ff53c 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1204,7 +1204,7 @@ main(int argc, char **argv) virBufferVSprintf(&buf, " \"%s/run/libvirt/**/%s.pid\" rwk,\n", LOCALSTATEDIR, ctl->def->name); if (ctl->files) - virBufferVSprintf(&buf, "%s", ctl->files); + virBufferAdd(&buf, ctl->files, -1); } if (virBufferError(&buf)) { diff --git a/src/util/sexpr.c b/src/util/sexpr.c index ae1692a..da3d4b3 100644 --- a/src/util/sexpr.c +++ b/src/util/sexpr.c @@ -1,7 +1,7 @@ /* * sexpr.c : S-Expression routines to communicate with the Xen Daemon * - * Copyright (C) 2010 Red Hat, Inc. + * Copyright (C) 2010-2011 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori <aliguori(a)us.ibm.com&gt; * * This file is subject to the terms and conditions of the GNU Lesser General @@ -234,7 +234,7 @@ sexpr2string(const struct sexpr *sexpr, virBufferPtr buffer) strchr(sexpr->u.value, '(')) virBufferVSprintf(buffer, "'%s'", sexpr->u.value); else - virBufferVSprintf(buffer, "%s", sexpr->u.value); + virBufferAdd(buffer, sexpr->u.value, -1); break; case SEXPR_NIL: diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 04122ba..57422d3 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -487,13 +487,11 @@ xend_op_ext(virConnectPtr xend, const char *path, const char *key, va_list ap) while (k) { v = va_arg(ap, const char *); - virBufferVSprintf(&buf, "%s", k); - virBufferVSprintf(&buf, "%s", "="); - virBufferVSprintf(&buf, "%s", v); + virBufferVSprintf(&buf, "%s=%s", k, v); k = va_arg(ap, const char *); if (k) - virBufferVSprintf(&buf, "%s", "&"); + virBufferAddChar(&buf, '&'); } if (virBufferError(&buf)) { diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index b590517..d2ec370 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1514,7 +1514,7 @@ xenFormatSxprChr(virDomainChrDefPtr def, case VIR_DOMAIN_CHR_TYPE_STDIO: case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_PTY: - virBufferVSprintf(buf, "%s", type); + virBufferAdd(buf, type, -1); break; case VIR_DOMAIN_CHR_TYPE_FILE: diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 89f75a5..dbcaf15 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1115,13 +1115,13 @@ static int xenFormatXMDisk(virConfValuePtr list, goto cleanup; } } - virBufferVSprintf(&buf, "%s", disk->src); + virBufferAdd(&buf, disk->src, -1); } virBufferAddLit(&buf, ","); if (hvm && xendConfigVersion == 1) virBufferAddLit(&buf, "ioemu:"); - virBufferVSprintf(&buf, "%s", disk->dst); + virBufferAdd(&buf, disk->dst, -1); if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) virBufferAddLit(&buf, ":cdrom"); -- 1.7.4.2

Ever since commit ebc46f, the destroy function built two command variants but only used one. I went with the variant that matches the idiom used in the counterpart of phypBuildStoragePool. * src/phyp/phyp_driver.c (phypDestroyStoragePool): Avoid clobbering cmd. Fix error message typo. --- src/phyp/phyp_driver.c | 11 +---------- 1 files changed, 1 insertions(+), 10 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 5742d95..7aa494d 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -2936,19 +2936,10 @@ phypDestroyStoragePool(virStoragePoolPtr pool) return -1; } cmd = virBufferContentAndReset(&buf); - - if (virAsprintf(&cmd, - "viosvrcmd -m %s --id %d -c " - "'rmsp %s'", managed_system, vios_id, - pool->name) < 0) { - virReportOOMError(); - goto cleanup; - } - ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0) { - VIR_ERROR(_("Unable to create Storage Pool: %s"), ret); + VIR_ERROR(_("Unable to destroy Storage Pool: %s"), ret); goto cleanup; } -- 1.7.4.2

* src/phyp/phyp_driver.c (phypUUIDTable_Init): Avoid memory leak on error. --- src/phyp/phyp_driver.c | 46 +++++++++++++++++++++++++++------------------- 1 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 7aa494d..0c69d4f 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -807,31 +807,35 @@ phypUUIDTable_Pull(virConnectPtr conn) static int phypUUIDTable_Init(virConnectPtr conn) { - uuid_tablePtr uuid_table; + uuid_tablePtr uuid_table = NULL; phyp_driverPtr phyp_driver; int nids_numdomains = 0; int nids_listdomains = 0; int *ids = NULL; unsigned int i = 0; + int ret = -1; + bool table_created = false; if ((nids_numdomains = phypNumDomainsGeneric(conn, 2)) < 0) - goto err; + goto cleanup; if (VIR_ALLOC_N(ids, nids_numdomains) < 0) { virReportOOMError(); - goto err; + goto cleanup; } if ((nids_listdomains = phypListDomainsGeneric(conn, ids, nids_numdomains, 1)) < 0) - goto err; + goto cleanup; /* exit early if there are no domains */ - if (nids_numdomains == 0 && nids_listdomains == 0) - goto exit; - else if (nids_numdomains != nids_listdomains) { + if (nids_numdomains == 0 && nids_listdomains == 0) { + ret = 0; + goto cleanup; + } + if (nids_numdomains != nids_listdomains) { VIR_ERROR0(_("Unable to determine number of domains.")); - goto err; + goto cleanup; } phyp_driver = conn->privateData; @@ -841,11 +845,12 @@ phypUUIDTable_Init(virConnectPtr conn) /* try to get the table from server */ if (phypUUIDTable_Pull(conn) == -1) { /* file not found in the server, creating a new one */ + table_created = true; if (VIR_ALLOC_N(uuid_table->lpars, uuid_table->nlpars) >= 0) { for (i = 0; i < uuid_table->nlpars; i++) { if (VIR_ALLOC(uuid_table->lpars[i]) < 0) { virReportOOMError(); - goto err; + goto cleanup; } uuid_table->lpars[i]->id = ids[i]; @@ -855,27 +860,30 @@ phypUUIDTable_Init(virConnectPtr conn) } } else { virReportOOMError(); - goto err; + goto cleanup; } if (phypUUIDTable_WriteFile(conn) == -1) - goto err; + goto cleanup; if (phypUUIDTable_Push(conn) == -1) - goto err; + goto cleanup; } else { if (phypUUIDTable_ReadFile(conn) == -1) - goto err; - goto exit; + goto cleanup; } - exit: - VIR_FREE(ids); - return 0; + ret = 0; - err: +cleanup: + if (ret < 0 && table_created) { + for (i = 0; i < uuid_table->nlpars; i++) { + VIR_FREE(uuid_table->lpars[i]); + } + VIR_FREE(uuid_table->lpars); + } VIR_FREE(ids); - return -1; + return ret; } static void -- 1.7.4.2

* src/phyp/phyp_driver.c (phypInterfaceDestroy) (phypInterfaceDefineXML, phypInterfaceLookupByName) (phypInterfaceIsActive, phypListInterfaces, phypNumOfInterfaces): Clean up return handling of recent additions. --- src/phyp/phyp_driver.c | 130 +++++++++++++++++++++--------------------------- 1 files changed, 56 insertions(+), 74 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 0c69d4f..37395c6 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3294,6 +3294,7 @@ phypInterfaceDestroy(virInterfacePtr iface, char *char_ptr; char *cmd = NULL; char *ret = NULL; + int rv = -1; /* Getting the remote slot number */ @@ -3309,17 +3310,17 @@ phypInterfaceDestroy(virInterfacePtr iface, if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - goto err; + goto cleanup; } cmd = virBufferContentAndReset(&buf); ret = phypExec(session, cmd, &exit_status, iface->conn); if (exit_status < 0 || ret == NULL) - goto err; + goto cleanup; if (virStrToLong_i(ret, &char_ptr, 10, &slot_num) == -1) - goto err; + goto cleanup; /* Getting the remote slot number */ VIR_FREE(cmd); @@ -3337,7 +3338,7 @@ phypInterfaceDestroy(virInterfacePtr iface, if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - goto err; + goto cleanup; } cmd = virBufferContentAndReset(&buf); @@ -3346,10 +3347,10 @@ phypInterfaceDestroy(virInterfacePtr iface, ret = phypExec(session, cmd, &exit_status, iface->conn); if (exit_status < 0 || ret == NULL) - goto err; + goto cleanup; if (virStrToLong_i(ret, &char_ptr, 10, &lpar_id) == -1) - goto err; + goto cleanup; /* excluding interface */ VIR_FREE(cmd); @@ -3366,23 +3367,21 @@ phypInterfaceDestroy(virInterfacePtr iface, if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - goto err; + goto cleanup; } cmd = virBufferContentAndReset(&buf); ret = phypExec(session, cmd, &exit_status, iface->conn); if (exit_status < 0 || ret != NULL) - goto err; + goto cleanup; - VIR_FREE(cmd); - VIR_FREE(ret); - return 0; + rv = 0; - err: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); - return -1; + return rv; } static virInterfacePtr @@ -3405,9 +3404,10 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, char name[PHYP_IFACENAME_SIZE]; char mac[PHYP_MAC_SIZE]; virInterfaceDefPtr def; + virInterfacePtr result = NULL; if (!(def = virInterfaceDefParseString(xml))) - goto err; + goto cleanup; /* Now need to get the next free slot number */ virBufferAddLit(&buf, "lshwres "); @@ -3422,17 +3422,17 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - goto err; + goto cleanup; } cmd = virBufferContentAndReset(&buf); ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0 || ret == NULL) - goto err; + goto cleanup; if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1) - goto err; + goto cleanup; /* The next free slot itself: */ slot++; @@ -3453,14 +3453,14 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - goto err; + goto cleanup; } cmd = virBufferContentAndReset(&buf); ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0 || ret != NULL) - goto err; + goto cleanup; /* Need to sleep a little while to wait for the HMC to * complete the execution of the command. @@ -3483,7 +3483,7 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - goto err; + goto cleanup; } cmd = virBufferContentAndReset(&buf); @@ -3505,13 +3505,13 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - goto err; + goto cleanup; } cmd = virBufferContentAndReset(&buf); ret = phypExec(session, cmd, &exit_status, conn); - goto err; + goto cleanup; } memcpy(name, ret, PHYP_IFACENAME_SIZE-1); @@ -3532,27 +3532,24 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - goto err; + goto cleanup; } cmd = virBufferContentAndReset(&buf); ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0 || ret == NULL) - goto err; + goto cleanup; memcpy(mac, ret, PHYP_MAC_SIZE-1); - VIR_FREE(cmd); - VIR_FREE(ret); - virInterfaceDefFree(def); - return virGetInterface(conn, name, mac); + result = virGetInterface(conn, name, mac); - err: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); virInterfaceDefFree(def); - return NULL; + return result; } static virInterfacePtr @@ -3586,17 +3583,17 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - goto err; + goto cleanup; } cmd = virBufferContentAndReset(&buf); ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0 || ret == NULL) - goto err; + goto cleanup; if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1) - goto err; + goto cleanup; /*Getting the lpar_id for the interface */ VIR_FREE(cmd); @@ -3621,10 +3618,10 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0 || ret == NULL) - goto err; + goto cleanup; if (virStrToLong_i(ret, &char_ptr, 10, &lpar_id) == -1) - goto err; + goto cleanup; /*Getting the interface mac */ virBufferAddLit(&buf, "lshwres "); @@ -3639,7 +3636,7 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - return NULL; + goto cleanup; } cmd = virBufferContentAndReset(&buf); @@ -3648,21 +3645,16 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0 || ret == NULL) - goto err; + goto cleanup; memcpy(mac, ret, PHYP_MAC_SIZE-1); result = virGetInterface(conn, name, ret); +cleanup: VIR_FREE(cmd); VIR_FREE(ret); return result; - - err: - VIR_FREE(cmd); - VIR_FREE(ret); - return NULL; - } static int @@ -3675,7 +3667,7 @@ phypInterfaceIsActive(virInterfacePtr iface) char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int exit_status = 0; - int state = 0; + int state = -1; char *char_ptr; char *cmd = NULL; char *ret = NULL; @@ -3692,27 +3684,22 @@ phypInterfaceIsActive(virInterfacePtr iface) if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - return -1; + goto cleanup; } cmd = virBufferContentAndReset(&buf); ret = phypExec(session, cmd, &exit_status, iface->conn); if (exit_status < 0 || ret == NULL) - goto err; + goto cleanup; if (virStrToLong_i(ret, &char_ptr, 10, &state) == -1) - goto err; + goto cleanup; +cleanup: VIR_FREE(cmd); VIR_FREE(ret); return state; - - err: - VIR_FREE(cmd); - VIR_FREE(ret); - return -1; - } static int @@ -3732,6 +3719,7 @@ phypListInterfaces(virConnectPtr conn, char **const names, int nnames) char *networks = NULL; char *char_ptr2 = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; + bool success = false; virBufferAddLit(&buf, "lshwres"); if (system_type == HMC) @@ -3742,15 +3730,16 @@ phypListInterfaces(virConnectPtr conn, char **const names, int nnames) if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - goto err; + goto cleanup; } cmd = virBufferContentAndReset(&buf); ret = phypExec(session, cmd, &exit_status, conn); - /* I need to parse the textual return in order to get the network interfaces */ + /* I need to parse the textual return in order to get the network + * interfaces */ if (exit_status < 0 || ret == NULL) - goto err; + goto cleanup; networks = ret; @@ -3761,7 +3750,7 @@ phypListInterfaces(virConnectPtr conn, char **const names, int nnames) *char_ptr2 = '\0'; if ((names[got++] = strdup(networks)) == NULL) { virReportOOMError(); - goto err; + goto cleanup; } char_ptr2++; networks = char_ptr2; @@ -3770,16 +3759,14 @@ phypListInterfaces(virConnectPtr conn, char **const names, int nnames) } } +cleanup: + if (!success) { + for (i = 0; i < got; i++) + VIR_FREE(names[i]); + } VIR_FREE(cmd); VIR_FREE(ret); return got; - - err: - for (i = 0; i < got; i++) - VIR_FREE(names[i]); - VIR_FREE(cmd); - VIR_FREE(ret); - return -1; } static int @@ -3792,7 +3779,7 @@ phypNumOfInterfaces(virConnectPtr conn) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - int nnets = 0; + int nnets = -1; char *char_ptr; char *cmd = NULL; char *ret = NULL; @@ -3809,27 +3796,22 @@ phypNumOfInterfaces(virConnectPtr conn) if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - goto err; + goto cleanup; } cmd = virBufferContentAndReset(&buf); ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0 || ret == NULL) - goto err; + goto cleanup; if (virStrToLong_i(ret, &char_ptr, 10, &nnets) == -1) - goto err; + goto cleanup; +cleanup: VIR_FREE(cmd); VIR_FREE(ret); return nnets; - - err: - VIR_FREE(cmd); - VIR_FREE(ret); - return -1; - } static int -- 1.7.4.2

* src/phyp/phyp_driver.c: Match label style of rest of project. (phypExec, phypUUIDTable_Pull): Drop an extra label. --- src/phyp/phyp_driver.c | 129 +++++++++++++++++++++++------------------------- 1 files changed, 62 insertions(+), 67 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 37395c6..a952875 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -182,15 +182,6 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, (*exit_status) = exitcode; libssh2_channel_free(channel); channel = NULL; - goto exit; - - err: - (*exit_status) = SSH_CMD_ERR; - virBufferFreeAndReset(&tex_ret); - VIR_FREE(buffer); - return NULL; - - exit: VIR_FREE(buffer); if (virBufferError(&tex_ret)) { @@ -199,6 +190,12 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, return NULL; } return virBufferContentAndReset(&tex_ret); + +err: + (*exit_status) = SSH_CMD_ERR; + virBufferFreeAndReset(&tex_ret); + VIR_FREE(buffer); + return NULL; } static int @@ -256,7 +253,7 @@ phypGetVIOSPartitionID(virConnectPtr conn) if (virStrToLong_i(ret, &char_ptr, 10, &id) == -1) goto cleanup; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -302,7 +299,7 @@ phypCapsInit(void) return caps; - no_memory: +no_memory: virCapabilitiesFree(caps); return NULL; } @@ -362,7 +359,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) if (virStrToLong_i(ret, &char_ptr, 10, &ndom) == -1) goto cleanup; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -430,7 +427,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, line++; /* skip \n */ } - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -471,7 +468,7 @@ phypUUIDTable_WriteFile(virConnectPtr conn) } return 0; - err: +err: VIR_FORCE_CLOSE(fd); return -1; } @@ -569,7 +566,7 @@ phypUUIDTable_Push(virConnectPtr conn) virBufferFreeAndReset(&username); return 0; - err: +err: if (channel) { libssh2_channel_send_eof(channel); libssh2_channel_wait_eof(channel); @@ -602,7 +599,7 @@ phypUUIDTable_RemLpar(virConnectPtr conn, int id) return 0; - err: +err: return -1; } @@ -637,7 +634,7 @@ phypUUIDTable_AddLpar(virConnectPtr conn, unsigned char *uuid, int id) return 0; - err: +err: return -1; } @@ -686,7 +683,7 @@ phypUUIDTable_ReadFile(virConnectPtr conn) VIR_FORCE_CLOSE(fd); return 0; - err: +err: VIR_FORCE_CLOSE(fd); return -1; } @@ -780,9 +777,7 @@ phypUUIDTable_Pull(virConnectPtr conn) local_file); goto err; } - goto exit; - exit: if (channel) { libssh2_channel_send_eof(channel); libssh2_channel_wait_eof(channel); @@ -793,7 +788,7 @@ phypUUIDTable_Pull(virConnectPtr conn) virBufferFreeAndReset(&username); return 0; - err: +err: if (channel) { libssh2_channel_send_eof(channel); libssh2_channel_wait_eof(channel); @@ -1041,7 +1036,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, freeaddrinfo(ai); goto err; - connected: +connected: (*internal_socket) = sock; @@ -1074,7 +1069,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, NULL)) == LIBSSH2_ERROR_EAGAIN) ; - keyboard_interactive: +keyboard_interactive: if (rc == LIBSSH2_ERROR_SOCKET_NONE || rc == LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED || rc == LIBSSH2_ERROR_PUBLICKEY_UNVERIFIED) { @@ -1112,10 +1107,10 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, goto err; } - disconnect: +disconnect: libssh2_session_disconnect(session, "Disconnecting..."); libssh2_session_free(session); - err: +err: VIR_FREE(userhome); VIR_FREE(pubkey); VIR_FREE(pvtkey); @@ -1123,7 +1118,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, VIR_FREE(password); return NULL; - exit: +exit: VIR_FREE(userhome); VIR_FREE(pubkey); VIR_FREE(pvtkey); @@ -1235,7 +1230,7 @@ phypOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS; - failure: +failure: if (phyp_driver != NULL) { virCapabilitiesFree(phyp_driver->caps); VIR_FREE(phyp_driver->managed_system); @@ -1327,7 +1322,7 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, if (virStrToLong_i(ret, &char_ptr, 10, &lpar_id) == -1) goto cleanup; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -1370,7 +1365,7 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system, if (char_ptr) *char_ptr = '\0'; - cleanup: +cleanup: VIR_FREE(cmd); return ret; @@ -1450,7 +1445,7 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, if (virStrToLong_i(ret, &char_ptr, 10, &memory) == -1) goto cleanup; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -1498,7 +1493,7 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, if (virStrToLong_i(ret, &char_ptr, 10, &vcpus) == -1) goto cleanup; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -1572,7 +1567,7 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, if (virStrToLong_i(ret, &char_ptr, 10, &remote_slot) == -1) goto cleanup; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -1651,7 +1646,7 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, if (char_ptr) *char_ptr = '\0'; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -1697,7 +1692,7 @@ phypGetLparProfile(virConnectPtr conn, int lpar_id) if (char_ptr) *char_ptr = '\0'; - cleanup: +cleanup: VIR_FREE(cmd); return ret; @@ -1755,7 +1750,7 @@ phypGetVIOSNextSlotNumber(virConnectPtr conn) slot += 1; - cleanup: +cleanup: VIR_FREE(profile); VIR_FREE(cmd); VIR_FREE(ret); @@ -1869,7 +1864,7 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) result = 0; - cleanup: +cleanup: VIR_FREE(profile); VIR_FREE(vios_name); VIR_FREE(cmd); @@ -1923,7 +1918,7 @@ phypGetVIOSFreeSCSIAdapter(virConnectPtr conn) if (char_ptr) *char_ptr = '\0'; - cleanup: +cleanup: VIR_FREE(cmd); return ret; @@ -2133,7 +2128,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) result = 0; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); virDomainDeviceDefFree(dev); @@ -2191,7 +2186,7 @@ phypVolumeGetKey(virConnectPtr conn, const char *name) if (char_ptr) *char_ptr = '\0'; - cleanup: +cleanup: VIR_FREE(cmd); return ret; @@ -2242,7 +2237,7 @@ phypGetStoragePoolDevice(virConnectPtr conn, char *name) if (char_ptr) *char_ptr = '\0'; - cleanup: +cleanup: VIR_FREE(cmd); return ret; @@ -2290,7 +2285,7 @@ phypGetStoragePoolSize(virConnectPtr conn, char *name) if (virStrToLong_i(ret, &char_ptr, 10, &sp_size) == -1) goto cleanup; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -2341,7 +2336,7 @@ phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname, if (key == NULL) goto cleanup; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -2454,7 +2449,7 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, return vol; - err: +err: VIR_FREE(key); virStorageVolDefFree(voldef); virStoragePoolDefFree(spdef); @@ -2509,7 +2504,7 @@ phypVolumeGetPhysicalVolumeByStoragePool(virStorageVolPtr vol, char *sp) if (char_ptr) *char_ptr = '\0'; - cleanup: +cleanup: VIR_FREE(cmd); return ret; @@ -2567,7 +2562,7 @@ phypVolumeLookupByPath(virConnectPtr conn, const char *volname) vol = virGetStorageVol(conn, spname, volname, key); - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(spname); VIR_FREE(key); @@ -2619,7 +2614,7 @@ phypGetStoragePoolUUID(virConnectPtr conn, unsigned char *uuid, result = 0; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -2706,7 +2701,7 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags) return xml; - err: +err: return NULL; } @@ -2775,7 +2770,7 @@ phypVolumeGetPath(virStorageVolPtr vol) goto cleanup; } - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(sp); VIR_FREE(path); @@ -2849,7 +2844,7 @@ phypStoragePoolListVolumes(virStoragePoolPtr pool, char **const volumes, success = true; - cleanup: +cleanup: if (!success) { for (i = 0; i < got; i++) VIR_FREE(volumes[i]); @@ -2906,7 +2901,7 @@ phypStoragePoolNumOfVolumes(virStoragePoolPtr pool) /* We need to remove 2 line from the header text output */ nvolumes -= 2; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -2953,7 +2948,7 @@ phypDestroyStoragePool(virStoragePoolPtr pool) result = 0; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -3002,7 +2997,7 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) result = 0; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -3052,7 +3047,7 @@ phypNumOfStoragePools(virConnectPtr conn) if (virStrToLong_i(ret, &char_ptr, 10, &nsp) == -1) goto cleanup; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -3120,7 +3115,7 @@ phypListStoragePools(virConnectPtr conn, char **const pools, int npools) success = true; - cleanup: +cleanup: if (!success) { for (i = 0; i < got; i++) VIR_FREE(pools[i]); @@ -3186,7 +3181,7 @@ phypGetStoragePoolLookUpByUUID(virConnectPtr conn, } } - err: +err: VIR_FREE(local_uuid); VIR_FREE(pools); return NULL; @@ -3224,7 +3219,7 @@ phypStoragePoolCreateXML(virConnectPtr conn, return sp; - err: +err: virStoragePoolDefFree(def); if (sp) virUnrefStoragePool(sp); @@ -3272,7 +3267,7 @@ phypGetStoragePoolXMLDesc(virStoragePoolPtr pool, unsigned int flags) return virStoragePoolDefFormat(&def); - err: +err: return NULL; } @@ -3857,7 +3852,7 @@ phypGetLparState(virConnectPtr conn, unsigned int lpar_id) else if (STREQ(ret, "Shutting Down")) state = VIR_DOMAIN_SHUTDOWN; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); return state; @@ -3909,7 +3904,7 @@ phypDiskType(virConnectPtr conn, char *backing_device) else if (STREQ(ret, "FBPOOL")) disk_type = VIR_DOMAIN_DISK_TYPE_FILE; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); return disk_type; @@ -3989,7 +3984,7 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) success = true; - cleanup: +cleanup: if (!success) { for (i = 0; i < got; i++) VIR_FREE(names[i]); @@ -4054,7 +4049,7 @@ phypDomainLookupByID(virConnectPtr conn, int lpar_id) if (dom) dom->id = lpar_id; - cleanup: +cleanup: VIR_FREE(lpar_name); return dom; @@ -4107,7 +4102,7 @@ phypDomainDumpXML(virDomainPtr dom, int flags) return virDomainDefFormat(&def, flags); - err: +err: return NULL; } @@ -4144,7 +4139,7 @@ phypDomainResume(virDomainPtr dom) result = 0; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -4184,7 +4179,7 @@ phypDomainShutdown(virDomainPtr dom) result = 0; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -4250,7 +4245,7 @@ phypDomainDestroy(virDomainPtr dom) dom->id = -1; result = 0; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -4327,7 +4322,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) result = 0; - cleanup: +cleanup: VIR_FREE(cmd); VIR_FREE(ret); @@ -4381,7 +4376,7 @@ phypDomainCreateAndStart(virConnectPtr conn, return dom; - err: +err: virDomainDefFree(def); if (dom) virUnrefDomain(dom); -- 1.7.4.2

* src/phyp/phyp_driver.c (phypUUIDTable_AddLpar) (phypGetLparUUID, phypGetStoragePoolUUID, phypVolumeGetXMLDesc) (phypGetStoragePoolXMLDesc): Use faster method. --- src/phyp/phyp_driver.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index a952875..27536eb 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -624,7 +624,7 @@ phypUUIDTable_AddLpar(virConnectPtr conn, unsigned char *uuid, int id) } uuid_table->lpars[i]->id = id; - memmove(uuid_table->lpars[i]->uuid, uuid, VIR_UUID_BUFLEN); + memcpy(uuid_table->lpars[i]->uuid, uuid, VIR_UUID_BUFLEN); if (phypUUIDTable_WriteFile(conn) == -1) goto err; @@ -1388,7 +1388,7 @@ phypGetLparUUID(unsigned char *uuid, int lpar_id, virConnectPtr conn) for (i = 0; i < uuid_table->nlpars; i++) { if (lpars[i]->id == lpar_id) { - memmove(uuid, lpars[i]->uuid, VIR_UUID_BUFLEN); + memcpy(uuid, lpars[i]->uuid, VIR_UUID_BUFLEN); return 0; } } @@ -2609,7 +2609,7 @@ phypGetStoragePoolUUID(virConnectPtr conn, unsigned char *uuid, if (exit_status < 0 || ret == NULL) goto cleanup; - if (memmove(uuid, ret, VIR_UUID_BUFLEN) == NULL) + if (memcpy(uuid, ret, VIR_UUID_BUFLEN) == NULL) goto cleanup; result = 0; @@ -2657,7 +2657,7 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags) goto err; } - if (memmove(pool.uuid, sp->uuid, VIR_UUID_BUFLEN) == NULL) { + if (memcpy(pool.uuid, sp->uuid, VIR_UUID_BUFLEN) == NULL) { VIR_ERROR0(_("Unable to determine storage sp's uuid.")); goto err; } @@ -3241,7 +3241,7 @@ phypGetStoragePoolXMLDesc(virStoragePoolPtr pool, unsigned int flags) goto err; } - if (memmove(def.uuid, pool->uuid, VIR_UUID_BUFLEN) == NULL) { + if (memcpy(def.uuid, pool->uuid, VIR_UUID_BUFLEN) == NULL) { VIR_ERROR0(_("Unable to determine storage pool's uuid.")); goto err; } -- 1.7.4.2

Use the name 'ret' for all phypExec results, to make it easier to wrap phypExec. Don't allow a possibly NULL ret through printf. * src/phyp/phyp_driver.c (phypBuildVolume, phypDestroyStoragePool) (phypBuildStoragePool, phypBuildLpar): Avoid NULL dereference. (phypInterfaceDestroy): Avoid redundant free. (phypVolumeLookupByPath, phypVolumeGetPath): Use consistent naming. --- src/phyp/phyp_driver.c | 37 +++++++++++++++++-------------------- 1 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 27536eb..56909af 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -2327,7 +2327,7 @@ phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname, ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0) { - VIR_ERROR(_("Unable to create Volume: %s"), ret); + VIR_ERROR(_("Unable to create Volume: %s"), NULLSTR(ret)); goto cleanup; } @@ -2521,7 +2521,7 @@ phypVolumeLookupByPath(virConnectPtr conn, const char *volname) int vios_id = phyp_driver->vios_id; int exit_status = 0; char *cmd = NULL; - char *spname = NULL; + char *ret = NULL; char *char_ptr; char *key = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2545,12 +2545,12 @@ phypVolumeLookupByPath(virConnectPtr conn, const char *volname) } cmd = virBufferContentAndReset(&buf); - spname = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); - if (exit_status < 0 || spname == NULL) + if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(spname, '\n'); + char_ptr = strchr(ret, '\n'); if (char_ptr) *char_ptr = '\0'; @@ -2560,11 +2560,11 @@ phypVolumeLookupByPath(virConnectPtr conn, const char *volname) if (key == NULL) goto cleanup; - vol = virGetStorageVol(conn, spname, volname, key); + vol = virGetStorageVol(conn, ret, volname, key); cleanup: VIR_FREE(cmd); - VIR_FREE(spname); + VIR_FREE(ret); VIR_FREE(key); return vol; @@ -2725,7 +2725,7 @@ phypVolumeGetPath(virStorageVolPtr vol) int vios_id = phyp_driver->vios_id; int exit_status = 0; char *cmd = NULL; - char *sp = NULL; + char *ret = NULL; char *path = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char *char_ptr; @@ -2750,33 +2750,32 @@ phypVolumeGetPath(virStorageVolPtr vol) } cmd = virBufferContentAndReset(&buf); - sp = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); - if (exit_status < 0 || sp == NULL) + if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(sp, '\n'); + char_ptr = strchr(ret, '\n'); if (char_ptr) *char_ptr = '\0'; - pv = phypVolumeGetPhysicalVolumeByStoragePool(vol, sp); + pv = phypVolumeGetPhysicalVolumeByStoragePool(vol, ret); if (!pv) goto cleanup; - if (virAsprintf(&path, "/%s/%s/%s", pv, sp, vol->name) < 0) { + if (virAsprintf(&path, "/%s/%s/%s", pv, ret, vol->name) < 0) { virReportOOMError(); goto cleanup; } cleanup: VIR_FREE(cmd); - VIR_FREE(sp); + VIR_FREE(ret); VIR_FREE(path); return path; - } static int @@ -2942,7 +2941,7 @@ phypDestroyStoragePool(virStoragePoolPtr pool) ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0) { - VIR_ERROR(_("Unable to destroy Storage Pool: %s"), ret); + VIR_ERROR(_("Unable to destroy Storage Pool: %s"), NULLSTR(ret)); goto cleanup; } @@ -2991,7 +2990,7 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0) { - VIR_ERROR(_("Unable to create Storage Pool: %s"), ret); + VIR_ERROR(_("Unable to create Storage Pool: %s"), NULLSTR(ret)); goto cleanup; } @@ -3337,8 +3336,6 @@ phypInterfaceDestroy(virInterfacePtr iface, } cmd = virBufferContentAndReset(&buf); - VIR_FREE(ret); - ret = phypExec(session, cmd, &exit_status, iface->conn); if (exit_status < 0 || ret == NULL) @@ -4311,7 +4308,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0) { - VIR_ERROR(_("Unable to create LPAR. Reason: '%s'"), ret); + VIR_ERROR(_("Unable to create LPAR. Reason: '%s'"), NULLSTR(ret)); goto cleanup; } -- 1.7.4.2

* src/phyp/phyp_driver.c (phypExecBuffer): New function. Use it throughout file for less code, and for plugging a few leaks. --- Amazing how much copy-and-paste code this consolidated. src/phyp/phyp_driver.c | 803 ++++++------------------------------------------ 1 files changed, 92 insertions(+), 711 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 56909af..bc24b76 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -110,6 +110,9 @@ waitsocket(int socket_fd, LIBSSH2_SESSION * session) /* this function is the layer that manipulates the ssh channel itself * and executes the commands on the remote machine */ +static char *phypExec(LIBSSH2_SESSION *, const char *, int *, virConnectPtr) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4); static char * phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, virConnectPtr conn) @@ -198,6 +201,33 @@ err: return NULL; } +/* Convenience wrapper function */ +static char *phypExecBuffer(LIBSSH2_SESSION *, virBufferPtr buf, int *, + virConnectPtr, bool) ATTRIBUTE_NONNULL(1) + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +static char * +phypExecBuffer(LIBSSH2_SESSION *session, virBufferPtr buf, int *exit_status, + virConnectPtr conn, bool strip_newline) +{ + char *cmd; + char *ret; + + if (virBufferError(buf)) { + virBufferFreeAndReset(buf); + virReportOOMError(); + return NULL; + } + cmd = virBufferContentAndReset(buf); + ret = phypExec(session, cmd, exit_status, conn); + VIR_FREE(cmd); + if (ret && *exit_status == 0 && strip_newline) { + char *nl = strchr(ret, '\n'); + if (nl) + *nl = '\0'; + } + return ret; +} + static int phypGetSystemType(virConnectPtr conn) { @@ -225,7 +255,6 @@ phypGetVIOSPartitionID(virConnectPtr conn) phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; int id = -1; @@ -238,14 +267,7 @@ phypGetVIOSPartitionID(virConnectPtr conn) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferAddLit(&buf, " -r lpar -F lpar_id,lpar_env" "|sed -n '/vioserver/ {\n s/,.*$//\n p\n}'"); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -254,7 +276,6 @@ phypGetVIOSPartitionID(virConnectPtr conn) goto cleanup; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return id; @@ -322,7 +343,6 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) int exit_status = 0; int ndom = -1; char *char_ptr; - char *cmd = NULL; char *ret = NULL; char *managed_system = phyp_driver->managed_system; const char *state; @@ -344,14 +364,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -F lpar_id,state %s |grep -c '^[0-9]*'", state); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -360,7 +373,6 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) goto cleanup; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return ndom; @@ -384,7 +396,6 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, char *managed_system = phyp_driver->managed_system; int exit_status = 0; int got = -1; - char *cmd = NULL; char *ret = NULL; char *line, *next_line; const char *state; @@ -400,14 +411,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -F lpar_id,state %s | sed -e 's/,.*$//'", state); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -428,9 +432,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, } cleanup: - VIR_FREE(cmd); VIR_FREE(ret); - return got; } @@ -1299,7 +1301,6 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, int exit_status = 0; int lpar_id = -1; char *char_ptr; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -1307,14 +1308,7 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " --filter lpar_names=%s -F lpar_id", name); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -1323,7 +1317,6 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, goto cleanup; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return lpar_id; @@ -1336,38 +1329,18 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system, { phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; virBufferAddLit(&buf, "lssyscfg -r lpar"); if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " --filter lpar_ids=%d -F name", lpar_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); - if (exit_status < 0 || ret == NULL) { + if (exit_status < 0) VIR_FREE(ret); - goto cleanup; - } - - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - -cleanup: - VIR_FREE(cmd); - return ret; } @@ -1409,7 +1382,6 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; char *char_ptr; int memory = 0; @@ -1425,28 +1397,15 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, virBufferVSprintf(&buf, " -r mem --level lpar -F %s --filter lpar_ids=%d", type ? "curr_mem" : "curr_max_mem", lpar_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return 0; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (virStrToLong_i(ret, &char_ptr, 10, &memory) == -1) goto cleanup; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return memory; @@ -1460,7 +1419,6 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; char *char_ptr; int exit_status = 0; @@ -1473,28 +1431,15 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, virBufferVSprintf(&buf, " -r proc --level lpar -F %s --filter lpar_ids=%d", type ? "curr_max_procs" : "curr_procs", lpar_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return 0; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (virStrToLong_i(ret, &char_ptr, 10, &vcpus) == -1) goto cleanup; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return vcpus; @@ -1535,7 +1480,6 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; char *char_ptr; int remote_slot = -1; @@ -1547,28 +1491,15 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -r virtualio --rsubtype scsi -F " "remote_slot_num --filter lpar_names=%s", lpar_name); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (virStrToLong_i(ret, &char_ptr, 10, &remote_slot) == -1) goto cleanup; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return remote_slot; @@ -1585,7 +1516,6 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; int remote_slot = 0; int exit_status = 0; @@ -1602,14 +1532,7 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -r virtualio --rsubtype scsi -F " "backing_devices --filter slots=%d", remote_slot); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -1647,7 +1570,6 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, *char_ptr = '\0'; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return backing_device; @@ -1662,10 +1584,8 @@ phypGetLparProfile(virConnectPtr conn, int lpar_id) char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; virBufferAddLit(&buf, "lssyscfg"); if (system_type == HMC) @@ -1673,28 +1593,10 @@ phypGetLparProfile(virConnectPtr conn, int lpar_id) virBufferVSprintf(&buf, " -r prof --filter lpar_ids=%d -F name|head -n 1", lpar_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); - ret = phypExec(session, cmd, &exit_status, conn); - - if (exit_status < 0 || ret == NULL) { + if (exit_status < 0) VIR_FREE(ret); - goto cleanup; - } - - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - -cleanup: - VIR_FREE(cmd); - return ret; } @@ -1709,7 +1611,6 @@ phypGetVIOSNextSlotNumber(virConnectPtr conn) int vios_id = phyp_driver->vios_id; int exit_status = 0; char *char_ptr; - char *cmd = NULL; char *ret = NULL; char *profile = NULL; int slot = -1; @@ -1731,16 +1632,7 @@ phypGetVIOSNextSlotNumber(virConnectPtr conn) "virtual_serial_adapters|sed -e 's/\"//g' -e " "'s/,/\\n/g'|sed -e 's/\\(^[0-9][0-9]\\*\\).*$/\\1/'" "|sort|tail -n 1", profile); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -1752,7 +1644,6 @@ phypGetVIOSNextSlotNumber(virConnectPtr conn) cleanup: VIR_FREE(profile); - VIR_FREE(cmd); VIR_FREE(ret); return slot; @@ -1769,7 +1660,6 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; char *profile = NULL; int slot = 0; @@ -1802,14 +1692,7 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) virBufferVSprintf(&buf, " -r prof --filter lpar_ids=%d,profile_names=%s" " -F virtual_scsi_adapters|sed -e s/\\\"//g", vios_id, profile); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -1823,17 +1706,8 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) virBufferVSprintf(&buf, " -r prof -i 'name=%s,lpar_id=%d," "\"virtual_scsi_adapters=%s,%d/server/any/any/1\"'", vios_name, vios_id, ret, slot); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - - VIR_FREE(cmd); VIR_FREE(ret); - - cmd = virBufferContentAndReset(&buf); - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -1847,17 +1721,7 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) virBufferVSprintf(&buf, " -p %s -o a -s %d -d 0 -a \"adapter_type=server\"", vios_name, slot); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - - VIR_FREE(cmd); - VIR_FREE(ret); - - cmd = virBufferContentAndReset(&buf); - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -1867,7 +1731,6 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) cleanup: VIR_FREE(profile); VIR_FREE(vios_name); - VIR_FREE(cmd); VIR_FREE(ret); return result; @@ -1883,10 +1746,8 @@ phypGetVIOSFreeSCSIAdapter(virConnectPtr conn) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; if (system_type == HMC) virBufferVSprintf(&buf, "viosvrcmd -m %s --id %d -c '", @@ -1898,29 +1759,10 @@ phypGetVIOSFreeSCSIAdapter(virConnectPtr conn) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed '/,[^.*]/d; s/,//g; q'"); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); - - if (exit_status < 0 || ret == NULL) { + if (exit_status < 0) VIR_FREE(ret); - goto cleanup; - } - - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - -cleanup: - VIR_FREE(cmd); - return ret; } @@ -1938,7 +1780,6 @@ phypAttachDevice(virDomainPtr domain, const char *xml) int vios_id = phyp_driver->vios_id; int exit_status = 0; char *char_ptr = NULL; - char *cmd = NULL; char *ret = NULL; char *scsi_adapter = NULL; int slot = 0; @@ -2000,18 +1841,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) if (system_type == HMC) virBufferAddChar(&buf, '\''); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - - VIR_FREE(cmd); - VIR_FREE(ret); - - cmd = virBufferContentAndReset(&buf); - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -2029,17 +1859,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) virBufferVSprintf(&buf, " slot_num,backing_device|grep %s|cut -d, -f1", dev->data.disk->src); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - - VIR_FREE(cmd); - VIR_FREE(ret); - - cmd = virBufferContentAndReset(&buf); - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -2057,17 +1877,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) " -r prof --filter lpar_ids=%d,profile_names=%s" " -F virtual_scsi_adapters|sed -e 's/\"//g'", vios_id, profile); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - - VIR_FREE(cmd); - VIR_FREE(ret); - - cmd = virBufferContentAndReset(&buf); - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -2083,17 +1893,8 @@ phypAttachDevice(virDomainPtr domain, const char *xml) "\"virtual_scsi_adapters=%s,%d/client/%d/%s/0\"'", domain_name, domain->id, ret, slot, vios_id, vios_name); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - - VIR_FREE(cmd); VIR_FREE(ret); - - cmd = virBufferContentAndReset(&buf); - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1) goto cleanup; @@ -2107,17 +1908,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) virBufferVSprintf(&buf, " -p %s -o a -s %d -d 0 -a \"adapter_type=server\"", domain_name, slot); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - - VIR_FREE(cmd); - VIR_FREE(ret); - - cmd = virBufferContentAndReset(&buf); - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) { VIR_ERROR0(_ @@ -2129,7 +1920,6 @@ phypAttachDevice(virDomainPtr domain, const char *xml) result = 0; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); virDomainDeviceDefFree(dev); virDomainDefFree(def); @@ -2151,10 +1941,8 @@ phypVolumeGetKey(virConnectPtr conn, const char *name) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; if (system_type == HMC) virBufferVSprintf(&buf, "viosvrcmd -m %s --id %d -c '", @@ -2166,29 +1954,10 @@ phypVolumeGetKey(virConnectPtr conn, const char *name) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed -e 's/^LV IDENTIFIER://' -e 's/ //g'"); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - - cmd = virBufferContentAndReset(&buf); - ret = phypExec(session, cmd, &exit_status, conn); - - if (exit_status < 0 || ret == NULL) { + if (exit_status < 0) VIR_FREE(ret); - goto cleanup; - } - - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - -cleanup: - VIR_FREE(cmd); - return ret; } @@ -2202,10 +1971,8 @@ phypGetStoragePoolDevice(virConnectPtr conn, char *name) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; if (system_type == HMC) virBufferVSprintf(&buf, "viosvrcmd -m %s --id %d -c '", @@ -2217,29 +1984,10 @@ phypGetStoragePoolDevice(virConnectPtr conn, char *name) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed '1d; s/ //g'"); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); - - if (exit_status < 0 || ret == NULL) { + if (exit_status < 0) VIR_FREE(ret); - goto cleanup; - } - - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - -cleanup: - VIR_FREE(cmd); - return ret; } @@ -2253,7 +2001,6 @@ phypGetStoragePoolSize(virConnectPtr conn, char *name) int system_type = phyp_driver->system_type; int exit_status = 0; int vios_id = phyp_driver->vios_id; - char *cmd = NULL; char *ret = NULL; int sp_size = -1; char *char_ptr; @@ -2269,15 +2016,7 @@ phypGetStoragePoolSize(virConnectPtr conn, char *name) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed '1d; s/ //g'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -2286,7 +2025,6 @@ phypGetStoragePoolSize(virConnectPtr conn, char *name) goto cleanup; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return sp_size; @@ -2302,7 +2040,6 @@ phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname, int vios_id = phyp_driver->vios_id; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2316,15 +2053,7 @@ phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname, if (system_type == HMC) virBufferAddChar(&buf, '\''); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - - cmd = virBufferContentAndReset(&buf); - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0) { VIR_ERROR(_("Unable to create Volume: %s"), NULLSTR(ret)); @@ -2333,11 +2062,7 @@ phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname, key = phypVolumeGetKey(conn, lvname); - if (key == NULL) - goto cleanup; - cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return key; @@ -2469,10 +2194,8 @@ phypVolumeGetPhysicalVolumeByStoragePool(virStorageVolPtr vol, char *sp) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; if (system_type == HMC) virBufferVSprintf(&buf, "viosvrcmd -m %s --id %d -c '", @@ -2484,29 +2207,10 @@ phypVolumeGetPhysicalVolumeByStoragePool(virStorageVolPtr vol, char *sp) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed 1d"); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); - - if (exit_status < 0 || ret == NULL) { + if (exit_status < 0) VIR_FREE(ret); - goto cleanup; - } - - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - -cleanup: - VIR_FREE(cmd); - return ret; } @@ -2520,9 +2224,7 @@ phypVolumeLookupByPath(virConnectPtr conn, const char *volname) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; - char *char_ptr; char *key = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; virStorageVolPtr vol = NULL; @@ -2537,24 +2239,11 @@ phypVolumeLookupByPath(virConnectPtr conn, const char *volname) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed -e 's/^VOLUME GROUP://g' -e 's/ //g'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - key = phypVolumeGetKey(conn, volname); if (key == NULL) @@ -2563,7 +2252,6 @@ phypVolumeLookupByPath(virConnectPtr conn, const char *volname) vol = virGetStorageVol(conn, ret, volname, key); cleanup: - VIR_FREE(cmd); VIR_FREE(ret); VIR_FREE(key); @@ -2582,7 +2270,6 @@ phypGetStoragePoolUUID(virConnectPtr conn, unsigned char *uuid, int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2596,15 +2283,7 @@ phypGetStoragePoolUUID(virConnectPtr conn, unsigned char *uuid, virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed '1,2d'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -2615,7 +2294,6 @@ phypGetStoragePoolUUID(virConnectPtr conn, unsigned char *uuid, result = 0; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return result; @@ -2724,11 +2402,9 @@ phypVolumeGetPath(virStorageVolPtr vol) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; char *path = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; char *pv; if (system_type == HMC) @@ -2742,24 +2418,11 @@ phypVolumeGetPath(virStorageVolPtr vol) virBufferVSprintf(&buf, "|sed -e 's/^VOLUME GROUP://g' -e 's/ //g'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - pv = phypVolumeGetPhysicalVolumeByStoragePool(vol, ret); if (!pv) @@ -2771,7 +2434,6 @@ phypVolumeGetPath(virStorageVolPtr vol) } cleanup: - VIR_FREE(cmd); VIR_FREE(ret); VIR_FREE(path); @@ -2793,7 +2455,6 @@ phypStoragePoolListVolumes(virStoragePoolPtr pool, char **const volumes, int exit_status = 0; int got = 0; int i; - char *cmd = NULL; char *ret = NULL; char *volumes_list = NULL; char *char_ptr2 = NULL; @@ -2809,15 +2470,7 @@ phypStoragePoolListVolumes(virStoragePoolPtr pool, char **const volumes, virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed '1,2d'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); /* I need to parse the textual return in order to get the volumes */ if (exit_status < 0 || ret == NULL) @@ -2850,10 +2503,7 @@ cleanup: got = -1; } - - VIR_FREE(cmd); VIR_FREE(ret); - return got; } @@ -2867,7 +2517,6 @@ phypStoragePoolNumOfVolumes(virStoragePoolPtr pool) int system_type = phyp_driver->system_type; int exit_status = 0; int nvolumes = -1; - char *cmd = NULL; char *ret = NULL; char *managed_system = phyp_driver->managed_system; int vios_id = phyp_driver->vios_id; @@ -2881,15 +2530,7 @@ phypStoragePoolNumOfVolumes(virStoragePoolPtr pool) if (system_type == HMC) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|grep -c '^.*$'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -2901,7 +2542,6 @@ phypStoragePoolNumOfVolumes(virStoragePoolPtr pool) nvolumes -= 2; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return nvolumes; @@ -2918,7 +2558,6 @@ phypDestroyStoragePool(virStoragePoolPtr pool) int vios_id = phyp_driver->vios_id; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2931,14 +2570,7 @@ phypDestroyStoragePool(virStoragePoolPtr pool) if (system_type == HMC) virBufferAddChar(&buf, '\''); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0) { VIR_ERROR(_("Unable to destroy Storage Pool: %s"), NULLSTR(ret)); @@ -2948,7 +2580,6 @@ phypDestroyStoragePool(virStoragePoolPtr pool) result = 0; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return result; @@ -2965,7 +2596,6 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) int vios_id = phyp_driver->vios_id; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2979,15 +2609,7 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) if (system_type == HMC) virBufferAddChar(&buf, '\''); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0) { VIR_ERROR(_("Unable to create Storage Pool: %s"), NULLSTR(ret)); @@ -2997,7 +2619,6 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) result = 0; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return result; @@ -3013,7 +2634,6 @@ phypNumOfStoragePools(virConnectPtr conn) int system_type = phyp_driver->system_type; int exit_status = 0; int nsp = -1; - char *cmd = NULL; char *ret = NULL; char *managed_system = phyp_driver->managed_system; int vios_id = phyp_driver->vios_id; @@ -3030,15 +2650,7 @@ phypNumOfStoragePools(virConnectPtr conn) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|grep -c '^.*$'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -3047,7 +2659,6 @@ phypNumOfStoragePools(virConnectPtr conn) goto cleanup; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return nsp; @@ -3066,7 +2677,6 @@ phypListStoragePools(virConnectPtr conn, char **const pools, int npools) int exit_status = 0; int got = 0; int i; - char *cmd = NULL; char *ret = NULL; char *storage_pools = NULL; char *char_ptr2 = NULL; @@ -3080,15 +2690,7 @@ phypListStoragePools(virConnectPtr conn, char **const pools, int npools) if (system_type == HMC) virBufferAddChar(&buf, '\''); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); /* I need to parse the textual return in order to get the storage pools */ if (exit_status < 0 || ret == NULL) @@ -3121,10 +2723,7 @@ cleanup: got = -1; } - - VIR_FREE(cmd); VIR_FREE(ret); - return got; } @@ -3286,7 +2885,6 @@ phypInterfaceDestroy(virInterfacePtr iface, int slot_num = 0; int lpar_id = 0; char *char_ptr; - char *cmd = NULL; char *ret = NULL; int rv = -1; @@ -3300,15 +2898,7 @@ phypInterfaceDestroy(virInterfacePtr iface, " -r virtualio --rsubtype eth --level lpar " " -F mac_addr,slot_num|" " sed -n '/%s/ s/^.*,//p'", iface->mac); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, iface->conn); + ret = phypExecBuffer(session, &buf, &exit_status, iface->conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -3317,7 +2907,6 @@ phypInterfaceDestroy(virInterfacePtr iface, goto cleanup; /* Getting the remote slot number */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "lshwres "); @@ -3328,15 +2917,7 @@ phypInterfaceDestroy(virInterfacePtr iface, " -r virtualio --rsubtype eth --level lpar " " -F mac_addr,lpar_id|" " sed -n '/%s/ s/^.*,//p'", iface->mac); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, iface->conn); + ret = phypExecBuffer(session, &buf, &exit_status, iface->conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -3345,7 +2926,6 @@ phypInterfaceDestroy(virInterfacePtr iface, goto cleanup; /* excluding interface */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "chhwres "); @@ -3355,15 +2935,7 @@ phypInterfaceDestroy(virInterfacePtr iface, virBufferVSprintf(&buf, " -r virtualio --rsubtype eth" " --id %d -o r -s %d", lpar_id, slot_num); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, iface->conn); + ret = phypExecBuffer(session, &buf, &exit_status, iface->conn, false); if (exit_status < 0 || ret != NULL) goto cleanup; @@ -3371,7 +2943,6 @@ phypInterfaceDestroy(virInterfacePtr iface, rv = 0; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return rv; } @@ -3390,7 +2961,6 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, int system_type = phyp_driver->system_type; int exit_status = 0; char *char_ptr; - char *cmd = NULL; int slot = 0; char *ret = NULL; char name[PHYP_IFACENAME_SIZE]; @@ -3410,15 +2980,7 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, " -r virtualio --rsubtype slot --level slot" " -Fslot_num --filter lpar_names=%s" " |sort|tail -n 1", def->name); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -3430,7 +2992,6 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, slot++; /* Now adding the new network interface */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "chhwres "); @@ -3441,15 +3002,7 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, " -r virtualio --rsubtype eth" " -p %s -o a -s %d -a port_vlan_id=1," "ieee_virtual_eth=0", def->name, slot); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret != NULL) goto cleanup; @@ -3460,7 +3013,6 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, sleep(1); /* Getting the new interface name */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "lshwres "); @@ -3471,19 +3023,10 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, " -r virtualio --rsubtype slot --level slot" " |sed '/lpar_name=%s/!d; /slot_num=%d/!d; " "s/^.*drc_name=//'", def->name, slot); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) { /* roll back and excluding interface if error*/ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "chhwres "); @@ -3493,23 +3036,13 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, virBufferVSprintf(&buf, " -r virtualio --rsubtype eth" " -p %s -o r -s %d", def->name, slot); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); goto cleanup; } memcpy(name, ret, PHYP_IFACENAME_SIZE-1); /* Getting the new interface mac addr */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "lshwres "); @@ -3520,15 +3053,7 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, "-r virtualio --rsubtype eth --level lpar " " |sed '/lpar_name=%s/!d; /slot_num=%d/!d; " "s/^.*mac_addr=//'", def->name, slot); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -3538,7 +3063,6 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, result = virGetInterface(conn, name, mac); cleanup: - VIR_FREE(cmd); VIR_FREE(ret); virInterfaceDefFree(def); return result; @@ -3555,7 +3079,6 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) int system_type = phyp_driver->system_type; int exit_status = 0; char *char_ptr; - char *cmd = NULL; char *ret = NULL; int slot = 0; int lpar_id = 0; @@ -3571,15 +3094,7 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) " -r virtualio --rsubtype slot --level slot " " -F drc_name,slot_num |" " sed -n '/%s/ s/^.*,//p'", name); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -3588,7 +3103,6 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) goto cleanup; /*Getting the lpar_id for the interface */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "lshwres "); @@ -3599,15 +3113,7 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) " -r virtualio --rsubtype slot --level slot " " -F drc_name,lpar_id |" " sed -n '/%s/ s/^.*,//p'", name); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -3624,17 +3130,8 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) " -r virtualio --rsubtype eth --level lpar " " -F lpar_id,slot_num,mac_addr|" " sed -n '/%d,%d/ s/^.*,//p'", lpar_id, slot); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - VIR_FREE(ret); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -3644,7 +3141,6 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) result = virGetInterface(conn, name, ret); cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return result; } @@ -3661,7 +3157,6 @@ phypInterfaceIsActive(virInterfacePtr iface) int exit_status = 0; int state = -1; char *char_ptr; - char *cmd = NULL; char *ret = NULL; virBufferAddLit(&buf, "lshwres "); @@ -3672,15 +3167,7 @@ phypInterfaceIsActive(virInterfacePtr iface) " -r virtualio --rsubtype eth --level lpar " " -F mac_addr,state |" " sed -n '/%s/ s/^.*,//p'", iface->mac); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, iface->conn); + ret = phypExecBuffer(session, &buf, &exit_status, iface->conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -3689,7 +3176,6 @@ phypInterfaceIsActive(virInterfacePtr iface) goto cleanup; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return state; } @@ -3706,7 +3192,6 @@ phypListInterfaces(virConnectPtr conn, char **const names, int nnames) int exit_status = 0; int got = 0; int i; - char *cmd = NULL; char *ret = NULL; char *networks = NULL; char *char_ptr2 = NULL; @@ -3719,14 +3204,7 @@ phypListInterfaces(virConnectPtr conn, char **const names, int nnames) virBufferVSprintf(&buf, " -r virtualio --rsubtype slot --level slot|" " sed '/eth/!d; /lpar_id=%d/d; s/^.*drc_name=//g'", vios_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); /* I need to parse the textual return in order to get the network * interfaces */ @@ -3756,7 +3234,6 @@ cleanup: for (i = 0; i < got; i++) VIR_FREE(names[i]); } - VIR_FREE(cmd); VIR_FREE(ret); return got; } @@ -3773,7 +3250,6 @@ phypNumOfInterfaces(virConnectPtr conn) int exit_status = 0; int nnets = -1; char *char_ptr; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -3784,15 +3260,7 @@ phypNumOfInterfaces(virConnectPtr conn) virBufferVSprintf(&buf, "-r virtualio --rsubtype eth --level lpar|" "grep -v lpar_id=%d|grep -c lpar_name", vios_id); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto cleanup; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto cleanup; @@ -3801,7 +3269,6 @@ phypNumOfInterfaces(virConnectPtr conn) goto cleanup; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return nnets; } @@ -3813,10 +3280,8 @@ phypGetLparState(virConnectPtr conn, unsigned int lpar_id) phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; - char *char_ptr = NULL; char *managed_system = phyp_driver->managed_system; int state = VIR_DOMAIN_NOSTATE; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -3825,23 +3290,11 @@ phypGetLparState(virConnectPtr conn, unsigned int lpar_id) if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -F state --filter lpar_ids=%d", lpar_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return state; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (STREQ(ret, "Running")) state = VIR_DOMAIN_RUNNING; else if (STREQ(ret, "Not Activated")) @@ -3850,7 +3303,6 @@ phypGetLparState(virConnectPtr conn, unsigned int lpar_id) state = VIR_DOMAIN_SHUTDOWN; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return state; } @@ -3864,10 +3316,8 @@ phypDiskType(virConnectPtr conn, char *backing_device) ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; - char *char_ptr; char *managed_system = phyp_driver->managed_system; int vios_id = phyp_driver->vios_id; int disk_type = -1; @@ -3879,30 +3329,17 @@ phypDiskType(virConnectPtr conn, char *backing_device) virBufferVSprintf(&buf, " -p %d -c \"lssp -field name type " "-fmt , -all|sed -n '/%s/ {\n s/^.*,//\n p\n}'\"", vios_id, backing_device); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return disk_type; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (STREQ(ret, "LVPOOL")) disk_type = VIR_DOMAIN_DISK_TYPE_BLOCK; else if (STREQ(ret, "FBPOOL")) disk_type = VIR_DOMAIN_DISK_TYPE_FILE; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return disk_type; } @@ -3937,7 +3374,6 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) int exit_status = 0; int got = 0; int i; - char *cmd = NULL; char *ret = NULL; char *domains = NULL; char *char_ptr2 = NULL; @@ -3948,14 +3384,7 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -F name,state" "|sed -n '/Not Activated/ {\n s/,.*$//\n p\n}'"); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); /* I need to parse the textual return in order to get the domains */ if (exit_status < 0 || ret == NULL) @@ -3988,10 +3417,7 @@ cleanup: got = -1; } - - VIR_FREE(cmd); VIR_FREE(ret); - return got; } @@ -4113,7 +3539,6 @@ phypDomainResume(virDomainPtr dom) int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4122,14 +3547,7 @@ phypDomainResume(virDomainPtr dom) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -r lpar -o on --id %d -f %s", dom->id, dom->name); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExecBuffer(session, &buf, &exit_status, dom->conn, false); if (exit_status < 0) goto cleanup; @@ -4137,7 +3555,6 @@ phypDomainResume(virDomainPtr dom) result = 0; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return result; @@ -4154,7 +3571,6 @@ phypDomainShutdown(virDomainPtr dom) int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4162,14 +3578,7 @@ phypDomainShutdown(virDomainPtr dom) if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -r lpar -o shutdown --id %d", dom->id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExecBuffer(session, &buf, &exit_status, dom->conn, false); if (exit_status < 0) goto cleanup; @@ -4177,7 +3586,6 @@ phypDomainShutdown(virDomainPtr dom) result = 0; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return result; @@ -4216,7 +3624,6 @@ phypDomainDestroy(virDomainPtr dom) int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4224,14 +3631,7 @@ phypDomainDestroy(virDomainPtr dom) if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -r lpar --id %d", dom->id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExecBuffer(session, &buf, &exit_status, dom->conn, false); if (exit_status < 0) goto cleanup; @@ -4243,7 +3643,6 @@ phypDomainDestroy(virDomainPtr dom) result = 0; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return result; @@ -4258,7 +3657,6 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4298,14 +3696,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) def->name, (int) def->mem.cur_balloon, (int) def->mem.cur_balloon, (int) def->mem.max_balloon, (int) def->vcpus, def->disks[0]->src); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0) { VIR_ERROR(_("Unable to create LPAR. Reason: '%s'"), NULLSTR(ret)); @@ -4320,7 +3711,6 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) result = 0; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return result; @@ -4402,7 +3792,6 @@ phypDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; char operation; unsigned long ncpus = 0; @@ -4438,14 +3827,7 @@ phypDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, virBufferVSprintf(&buf, " --id %d -o %c --procunits %d 2>&1 |sed " "-e 's/^.*\\([0-9][0-9]*.[0-9][0-9]*\\).*$/\\1/'", dom->id, operation, amount); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return 0; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExecBuffer(session, &buf, &exit_status, dom->conn, false); if (exit_status < 0) { VIR_ERROR0(_ @@ -4453,7 +3835,6 @@ phypDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, " Contact your support to enable this feature.")); } - VIR_FREE(cmd); VIR_FREE(ret); return 0; -- 1.7.4.2

2011/4/14 Eric Blake <eblake(a)redhat.com&gt;: You made the parsing stricter by passing NULL as second argument to virStrToLong_i. I don't expect it but this might be possible that this breaks the behavior of the driver. The rest of the patch looks fine. Matthias

On 04/16/2011 12:18 AM, Matthias Bolte wrote: Thanks for doing that. Oops - that's a definite bug; elsewhere in the file, we consistently use "[0-9][0-9]*" when matching a non-empty string of digits ([0-9]+ is not portable to POSIX BRE). Here's what I'm squashing in. Do I need to send a v3, or is this interdiff sufficient for an ack? diff --git i/src/phyp/phyp_driver.c w/src/phyp/phyp_driver.c index fc4ad5c..6bb9b49 100644 --- i/src/phyp/phyp_driver.c +++ w/src/phyp/phyp_driver.c @@ -237,13 +237,16 @@ phypExecInt(LIBSSH2_SESSION *session, virBufferPtr buf, virConnectPtr conn, { char *str; int ret; + char *char_ptr; str = phypExecBuffer(session, buf, &ret, conn, true); if (!str || ret) { VIR_FREE(str); return -1; } - ret = virStrToLong_i(str, NULL, 10, result); + ret = virStrToLong_i(str, &char_ptr, 10, result); + if (ret == 0 && *char_ptr) + VIR_WARN("ignoring suffix during integer parsing of '%s'", str); VIR_FREE(str); return ret; } @@ -366,7 +369,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) virBufferAddLit(&buf, "lssyscfg -r lpar"); if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); - virBufferVSprintf(&buf, " -F lpar_id,state %s |grep -c '^[0-9]*'", + virBufferVSprintf(&buf, " -F lpar_id,state %s |grep -c '^[0-9][0-9]*'", state); phypExecInt(session, &buf, conn, &ndom); return ndom; -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org