2011/4/14 Eric Blake <eblake@redhat.com>:
* 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(-)
@@ -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);
Why are you killing the VIR_FREE(ret) here, in the hunk before you left it in. It's necessary here too.
- 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;
@@ -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);
Same for this VIR_FREE(ret) here, removing it produces a leak, doesn't it.
- - 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);
And this VIR_FREE(ret) needs to stay too.
- - 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);
And this VIR_FREE(ret) needs to stay too.
- - 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);
This one is left in correctly.
- - 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);
But this VIR_FREE(ret) needs to stay.
- - 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(_
ACK, with that VIR_FREEs left in. Matthias