Hi JC, On Fri, 2018-11-09 at 08:46 -0800, JC Beyler wrote: > Hi Severin, > > The fix looks good to me.
Thanks for the review! > For reference for other reviewers, the original change can be seen > here: > http://hg.openjdk.java.net/jdk/jdk/rev/c0f9161f591e > > And it contains fixes from the following change (which puts a +1 to > avoid overflow): > http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/cd86b5699825 > > from the bug: > https://bugs.openjdk.java.net/browse/JDK-8140482 Thanks for these references. I should have put them into the review email right from the start. Cheers, Severin > Thanks, > Jc > > On Fri, Nov 9, 2018 at 8:03 AM Severin Gehwolf <sgehw...@redhat.com> > wrote: > > Hi, > > > > Could somebody please review this 8u backport of 8210836 as I'd > > like to > > get 8210647 (opt for sa) backported to 8u as well? Unfortunately > > the > > change from JDK 12 doesn't apply cleanly so I've included select > > changes from 8140482 so that the backport remains minimal. If > > anything, > > this makes the code more robust I'd think. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8210836 > > webrev: > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210836/jdk8/webrev.01/ > > > > Testing: Manual testing of loading core file in SA on linux. > > Stepping > > through code in the debugger. Basic jsadebugd core file loading. > > > > Thoughts? > > > > Thanks, > > Severin > > > > > >
