Looks good! Thanks, /Staffan
On 26 aug 2014, at 10:15, Jaroslav Bachorik <jaroslav.bacho...@oracle.com> wrote: > On 08/25/2014 08:37 PM, Staffan Larsen wrote: >> Thanks for taking on this one! >> >> The code looks good. Just two small things below. >> >> Have you tested with -Xverify:all, just to see if there are any byte code >> problems? > > I've re-run the tests with -Xverify:all and fixed one problem in the > generated bytecode. Thanks for reminding me. > >> >> >> Could fix the auto-naming of the params in this code? >> 131 @Override >> 132 public void visit(int i, int i1, String className, String >> string1, String string2, String[] strings) { >> 133 this.className = className; >> 134 super.visit(i, i1, className, string1, string2, >> strings); >> 135 } > > Done ... > >> >> nit: let’s ClassWriter to deal -> let ClassWriter deal >> 163 mv.visitMaxs(1, 1); // dummy call; let's >> ClassWriter to deal with this > > ... and done. > > http://cr.openjdk.java.net/~jbachorik/8037082/webrev.01 > > Cheers, > > -JB- > >> >> >> Thanks, >> /Staffan >> >> >> On 25 aug 2014, at 19:16, Jaroslav Bachorik <jaroslav.bacho...@oracle.com> >> wrote: >> >>> Please, review the following test fix. >>> >>> Issue : https://bugs.openjdk.java.net/browse/JDK-8037082 >>> Webrev: http://cr.openjdk.java.net/~jbachorik/8037082/webrev.00 >>> >>> As Staffan mentions in the issue comments - "The two tests >>> NativeMethodPrefixAgent and RetransformAgent use their own byte code >>> instrumentation library in jdk/test/java/lang/instrument/ilib/. These tests >>> need to be rewritten to use ASM instead so that we don't have to maintain >>> the ilib library." >>> >>> This patch is intended to remove the "ilib" library and replace the usages >>> with an ASM5 alternative. Only the currently used features of the "ilib" >>> library are being ported. >>> >>> Thanks, >>> >>> -JB- >> >