[libvirt] [PATCHv2 8/9] phyp: avoid memory leaks in command values
Matthias Bolte
matthias.bolte at googlemail.com
Fri Apr 15 20:56:19 UTC 2011
2011/4/14 Eric Blake <eblake at 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
More information about the libvir-list
mailing list