[libvirt] [PATCH] Fix a memory leak in the qemudBuildCommandLine.
ADD_ARG_LIT should only be used for literal arguments, since it duplicates the memory. Since virBufferContentAndReset is already allocating memory, we should only use ADD_ARG. Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/qemu/qemu_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 172986e..d0655fd 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -4110,7 +4110,7 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG_LIT(virBufferContentAndReset(&boot_buf)); + ADD_ARG(virBufferContentAndReset(&boot_buf)); } if (def->os.kernel) { -- 1.7.1.1
On 07/30/2010 07:47 AM, Chris Lalancette wrote:
ADD_ARG_LIT should only be used for literal arguments, since it duplicates the memory. Since virBufferContentAndReset is already allocating memory, we should only use ADD_ARG.
ACK. Hmm, wondering if there is a way to make use of gcc's __builtin_constant_p() to enforce that ADD_ARG_LIT is passed a string constant. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 07/30/10 - 08:02:08AM, Eric Blake wrote:
On 07/30/2010 07:47 AM, Chris Lalancette wrote:
ADD_ARG_LIT should only be used for literal arguments, since it duplicates the memory. Since virBufferContentAndReset is already allocating memory, we should only use ADD_ARG.
ACK.
Hmm, wondering if there is a way to make use of gcc's __builtin_constant_p() to enforce that ADD_ARG_LIT is passed a string constant.
Interesting idea. I know danpb wanted to remove all of the macros used in qemudBuildCommandLine, though, so you may want to consult with him before spending too much time on it. -- Chris Lalancette
On 07/30/2010 08:02 AM, Chris Lalancette wrote:
On 07/30/10 - 08:02:08AM, Eric Blake wrote:
On 07/30/2010 07:47 AM, Chris Lalancette wrote:
ADD_ARG_LIT should only be used for literal arguments, since it duplicates the memory. Since virBufferContentAndReset is already allocating memory, we should only use ADD_ARG.
ACK.
Hmm, wondering if there is a way to make use of gcc's __builtin_constant_p() to enforce that ADD_ARG_LIT is passed a string constant.
Interesting idea. I know danpb wanted to remove all of the macros used in qemudBuildCommandLine, though, so you may want to consult with him before spending too much time on it.
True - his pending exec library rewrite will override anything we do to the macros. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, Jul 30, 2010 at 10:02:51AM -0400, Chris Lalancette wrote:
On 07/30/10 - 08:02:08AM, Eric Blake wrote:
On 07/30/2010 07:47 AM, Chris Lalancette wrote:
ADD_ARG_LIT should only be used for literal arguments, since it duplicates the memory. Since virBufferContentAndReset is already allocating memory, we should only use ADD_ARG.
ACK.
Hmm, wondering if there is a way to make use of gcc's __builtin_constant_p() to enforce that ADD_ARG_LIT is passed a string constant.
Interesting idea. I know danpb wanted to remove all of the macros used in qemudBuildCommandLine, though, so you may want to consult with him before spending too much time on it.
Yes, don't bother changing this. The new APIs will *always* strdup all strings, since this optimization isn't useful in the real world Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (3)
-
Chris Lalancette -
Daniel P. Berrange -
Eric Blake