2011/4/14 Eric Blake <eblake@xxxxxxxxxx>:
> * 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list