[OpenJDK 2D-Dev] Request for review: 7151427: Potential memory leak in error handling code in X11SurfaceData.c

[Andrew Brygin]

Hello Sean,

  I have reviewed the change and it looks fine to me.

Thanks,
Andrew

On 07.03.2012 11:31, Sean Chou wrote:
> Hi all,
>
>     I still need one more review count, can any one help take a look ? 
> Thank you in advance !
>
> ---------- Forwarded message ----------
> From: *Sean Chou* <zhouyx at linux.vnet.ibm.com 
> <mailto:zhouyx at linux.vnet.ibm.com>>
> Date: Tue, Mar 6, 2012 at 5:07 PM
> Subject: Request for review: 7151427: Potential memory leak in error 
> handling code in X11SurfaceData.c
> To: 2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>
>
>
> Hi all,
>
>     The error handling code in X11SurfaceData.c does not free all the 
> memory allocated, it rarely causes problems because applications 
> almost never go there. However, we got one machine actually 
> encountered this problem.
>     Although there is no simple testcase for this bug, it is so 
> obvious that we just need read the code.
>
>     I reported a bug (7151427) for it and the webrev is at:
> http://cr.openjdk.java.net/~zhouyx/7151427/webrev.00/ 
> <http://cr.openjdk.java.net/%7Ezhouyx/7151427/webrev.00/>  .
>
>     Can anyone take a look ? Thanks.
>
> -- 
> Best Regards,
> Sean Chou
>
>
> On Wed, Mar 7, 2012 at 10:51 AM, Sean Chou <zhouyx at linux.vnet.ibm.com 
> <mailto:zhouyx at linux.vnet.ibm.com>> wrote:
>
>     Thanks for your comments. The part in  X11SD_DisposeXImage
>     is not necessary change to fix memory leak. It is just a little
>     clean up, as XDestroyImage and XCreateImage work in a pair.
>
>
>     On Wed, Mar 7, 2012 at 8:24 AM, Phil Race <philip.race at oracle.com
>     <mailto:philip.race at oracle.com>> wrote:
>
>         Looks OK to me.  Just one question. Was the part in
>         X11SD_DisposeXImage() a necessary change? My guess is
>         no and that you observed XDestroyImage() does the free()
>         and XFree() for us, so this is just to have some cleaner code.
>
>         Also you should try to find one more qualified reviewer as the
>         general client area policy is 2 reviewers.
>
>         -phil.
>
>
>
>         On 3/6/2012 1:07 AM, Sean Chou wrote:
>
>             Hi all,
>
>                The error handling code in X11SurfaceData.c does not
>             free all the memory allocated, it rarely causes problems
>             because applications almost never go there. However, we
>             got one machine actually encountered this problem.
>                Although there is no simple testcase for this bug, it
>             is so obvious that we just need read the code.
>
>                I reported a bug (7151427) for it and the webrev is at:
>             http://cr.openjdk.java.net/~zhouyx/7151427/webrev.00/
>             <http://cr.openjdk.java.net/%7Ezhouyx/7151427/webrev.00/>
>             <http://cr.openjdk.java.net/%7Ezhouyx/7151427/webrev.00/>  .
>
>
>                Can anyone take a look ? Thanks.
>
>             -- 
>             Best Regards,
>             Sean Chou
>
>
>
>
>
>     -- 
>     Best Regards,
>     Sean Chou
>
>
>
>
> -- 
> Best Regards,
> Sean Chou
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20120309/4e8cf9ac/attachment.html>