On Mon, Mar 21, 2011 at 11:52 AM, Ulrich Obergfell <uober...@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 <uober...@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
