| Message ID | 732683382.437646.1300708339597.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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); } }
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>