3:56 p.m.
2011/4/14 Eric Blake <eblake(a)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