On 04/05/2012 10:56 PM, Daniel Veillard wrote:
> On Thu, Apr 05, 2012 at 10:24:49PM -0600, Eric Blake wrote:
>> Leak introduced in commit 0436d32. If we allocate an actions array,
>> but fail early enough to never consume it with the qemu monitor
>> transaction call, we leaked memory.
>>
>> But our semantics of making the transaction command free the caller's
>> memory is awkward; avoiding the memory leak requires making every
>> intermediate function in the call chain check for error. It is much
>> easier to fix things so that the function that allocates also frees,
>> while the call chain leaves the caller's data intact. To do that,
>> I had to hack our JSON data structure to make it easy to protect a
>> portion of an arbitrary JSON tree from being freed.
>>
>
> ACK, sounds better to me than piggy-backing on an enum value :-)
Thanks. But I found one more (unlikely) issue:
qemuMonitorJSONMakeCommand can also trigger a recursive free on a
partial construction when hitting OOM. So I'm pushing with this
additional modification:
diff --git i/src/qemu/qemu_monitor_json.c w/src/qemu/qemu_monitor_json.c
index 8eeb307..d09d779 100644
--- i/src/qemu/qemu_monitor_json.c
+++ w/src/qemu/qemu_monitor_json.c
@@ -3160,16 +3160,18 @@ cleanup:
int
qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr actions)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
- bool protect;
+ bool protect = actions->protect;
+ /* We do NOT want to free actions when recursively freeing cmd. */
+ actions->protect = true;
cmd = qemuMonitorJSONMakeCommand("transaction",
"a:actions", actions,
NULL);
if (!cmd)
- return -1;
+ goto cleanup;
if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
goto cleanup;
@@ -3177,12 +3179,9 @@ qemuMonitorJSONTransaction(qemuMonitorPtr mon,
virJSONValuePtr actions)
ret = qemuMonitorJSONCheckError(cmd, reply);
cleanup:
- /* We do NOT want to free actions when recursively freeing cmd. */
- protect = actions->protect;
- actions->protect = true;
virJSONValueFree(cmd);
- actions->protect = protect;
virJSONValueFree(reply);
+ actions->protect = protect;
return ret;
}
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list