[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR 8037082: java/lang/instrument/NativeMethodPrefixAgent.java failing
From:       Staffan Larsen <staffan.larsen () oracle ! com>
Date:       2014-08-26 8:43:33
Message-ID: 45C03071-F283-4AE2-80FB-86362387C533 () oracle ! com
[Download RAW message or body]

Looks good!

Thanks,
/Staffan

On 26 aug 2014, at 10:15, Jaroslav Bachorik <jaroslav.bachorik@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.bachorik@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-
> > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]