qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] vnc: severe memory leak caused by broken palette_destroy() function
       [not found] <649134907.437547.1300708075324.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com>
@ 2011-03-21 11:52 ` Ulrich Obergfell
  2011-03-21 12:33   ` Stefan Hajnoczi
  0 siblings, 1 reply; 2+ messages in thread
From: Ulrich Obergfell @ 2011-03-21 11:52 UTC (permalink / raw)
  To: qemu-devel


The following commit breaks the code of the function palette_destroy().

http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commit;h=e31e3694afef58ba191cbcc6875ec243e5971268

The broken code causes a severe memory leak of 'VncPalette' structures
because it never frees anything:

     70 void palette_destroy(VncPalette *palette)
     71 {
     72     if (palette == NULL) {
     73         qemu_free(palette);
     74     }
     75 }

Calling qemu_free() unconditionally could be considered. However,
the original code (prior to the aforementioned commit) returned
immediately if 'palette' was NULL. In order to be closer to the
original code, the proposed patch corrects the 'if' statement.

Signed-off-by: Ulrich Obergfell <uobergfe@redhat.com>


diff -up ./ui/vnc-palette.c.orig0 ./ui/vnc-palette.c
--- ./ui/vnc-palette.c.orig0	2011-03-15 03:53:22.000000000 +0100
+++ ./ui/vnc-palette.c	2011-03-20 11:52:57.257560295 +0100
@@ -69,7 +69,7 @@ void palette_init(VncPalette *palette, s
 
 void palette_destroy(VncPalette *palette)
 {
-    if (palette == NULL) {
+    if (palette) {
         qemu_free(palette);
     }
 }

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Qemu-devel] vnc: severe memory leak caused by broken palette_destroy() function
  2011-03-21 11:52 ` [Qemu-devel] vnc: severe memory leak caused by broken palette_destroy() function Ulrich Obergfell
@ 2011-03-21 12:33   ` Stefan Hajnoczi
  0 siblings, 0 replies; 2+ messages in thread
From: Stefan Hajnoczi @ 2011-03-21 12:33 UTC (permalink / raw)
  To: Ulrich Obergfell; +Cc: qemu-devel

On Mon, Mar 21, 2011 at 11:52 AM, Ulrich Obergfell <uobergfe@redhat.com> wrote:
>
> The following commit breaks the code of the function palette_destroy().
>
> http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commit;h=e31e3694afef58ba191cbcc6875ec243e5971268
>
> The broken code causes a severe memory leak of 'VncPalette' structures
> because it never frees anything:
>
>     70 void palette_destroy(VncPalette *palette)
>     71 {
>     72     if (palette == NULL) {
>     73         qemu_free(palette);
>     74     }
>     75 }
>
> Calling qemu_free() unconditionally could be considered. However,
> the original code (prior to the aforementioned commit) returned
> immediately if 'palette' was NULL. In order to be closer to the
> original code, the proposed patch corrects the 'if' statement.
>
> Signed-off-by: Ulrich Obergfell <uobergfe@redhat.com>
>
>
> diff -up ./ui/vnc-palette.c.orig0 ./ui/vnc-palette.c
> --- ./ui/vnc-palette.c.orig0    2011-03-15 03:53:22.000000000 +0100
> +++ ./ui/vnc-palette.c  2011-03-20 11:52:57.257560295 +0100
> @@ -69,7 +69,7 @@ void palette_init(VncPalette *palette, s
>
>  void palette_destroy(VncPalette *palette)
>  {
> -    if (palette == NULL) {
> +    if (palette) {
>         qemu_free(palette);
>     }
>  }

Please drop the if (palette) check entirely because qemu_free(NULL) is
a nop.  There's no need to perform this check at all.

Stefan

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-03-21 12:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <649134907.437547.1300708075324.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com>
2011-03-21 11:52 ` [Qemu-devel] vnc: severe memory leak caused by broken palette_destroy() function Ulrich Obergfell
2011-03-21 12:33   ` Stefan Hajnoczi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).