Thanks to Andrew, Phil, and Jennifer ! And thanks to Charles for committing.
On Fri, Mar 9, 2012 at 11:43 PM, Andrew Brygin <andrew.bry...@oracle.com>wrote: > 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 <zho...@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@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/ . > > Can anyone take a look ? Thanks. > > -- > Best Regards, > Sean Chou > > > On Wed, Mar 7, 2012 at 10:51 AM, Sean Chou <zho...@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.r...@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/> . >>>> >>>> >>>> Can anyone take a look ? Thanks. >>>> >>>> -- >>>> Best Regards, >>>> Sean Chou >>>> >>>> >>> >> >> >> -- >> Best Regards, >> Sean Chou >> >> > > > -- > Best Regards, > Sean Chou > > > -- Best Regards, Sean Chou
