Re: [libvirt] [PATCH 2/6] qemu: Plug memory leak on qemuProcessWaitForMonitor() error path

At 11/30/2011 02:32 PM, Alex Jia Write: > On 11/30/2011 02:20 PM, Wen Congyang wrote: >> At 11/30/2011 01:57 PM, ajia(a)redhat.com Write: >>> From: Alex Jia&lt;ajia(a)redhat.com&gt; >>> >>> Detected by Coverity. Leak introduced in commit 109efd7. >>> >>> Signed-off-by: Alex Jia&lt;ajia(a)redhat.com&gt; >>> --- >>> src/qemu/qemu_process.c | 1 + >>> 1 files changed, 1 insertions(+), 0 deletions(-) >>> >>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c >>> index 2563f97..f3f44ca 100644 >>> --- a/src/qemu/qemu_process.c >>> +++ b/src/qemu/qemu_process.c >>> @@ -1224,6 +1224,7 @@ qemuProcessWaitForMonitor(struct qemud_driver* >>> driver, >>> >>> if (VIR_ALLOC_N(buf, buf_size)< 0) { >>> virReportOOMError(); >>> + VIR_FORCE_CLOSE(logfd); >>> return -1; >> I think it is better to goto closelog > Yeah, I think so before, but 'closelog' label will free 'buf', in fact, > we haven't successfully allocate buf is inited to NULL, so it is safe to use VIR_FREE(buf)

> memory to 'buf' variable, I'm not sure whether it is a issue. maybe, the > above is a simple way, otherwise, it should be better if we add a check > for 'buf' variable in 'closelog' label, it looks like this: > ... > if (buf) > VIR_FREE(buf); If buf is NULL, VIR_FREE(bus) does nothing and is safe. If you add a check for 'buf', make syntax-check will fail.

Thansk Wen Congyang > ... > > Chongyang, please correct me if I'm wrong :) > > Thanks, > Alex >> Thanks >> Wen Congyang >> >>> } >