RFR (M) 8211131: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/[G-I]*

Alex Menkov alexey.menkov at oracle.com
Tue Oct 2 21:03:46 UTC 2018


Hi Jc,

This looks much better to me :)
Sometimes the script generate too long lines like
-            if (!NSK_JNI_VERIFY(jni, (testedMeth = 
NSK_CPP_STUB4(GetStaticMethodID,
-                    jni, objCls, meth_sig[clsIdx][i][0],
-                    meth_sig[clsIdx][i][2])) != NULL)) {
+            if (!NSK_JNI_VERIFY(jni, (testedMeth = 
jni->GetStaticMethodID(objCls, meth_sig[clsIdx][i][0], 
meth_sig[clsIdx][i][2])) != NULL)) {

But I think it would be better to fix them clearing NSK_JNI_VERIFY stuff.

So LGTM.

--alex


On 10/02/2018 06:51, JC Beyler wrote:
> Hi Alex,
> 
> That is because this webrev was done before I added the 100 character 
> wrap, which I added when I was generating the next batch of changes. 
> Here is the webrev with the new version of the script:
> 
> Webrev: http://cr.openjdk.java.net/~jcbeyler/8211131/webrev.01 
> <http://cr.openjdk.java.net/%7Ejcbeyler/8211131/webrev.01>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8211131
> 
> Thanks, let me know what you think!
> Jc
> 
> On Tue, Oct 2, 2018 at 2:56 AM Alex Menkov <alexey.menkov at oracle.com 
> <mailto:alexey.menkov at oracle.com>> wrote:
> 
>     Hi Jc,
> 
>     Looking at your conversion script I expected things like
> 
>                if (!NSK_JVMTI_VERIFY(
>                      NSK_CPP_STUB2(AddCapabilities, jvmti, &caps))) {
> 
>     to be converted to
> 
>     if (!NSK_JVMTI_VERIFY(jvmti->AddCapabilities(&caps))) {
> 
>     (then the final string is shorter than 100 symbols)
> 
>     But actually I see
> 
>     if (!NSK_JVMTI_VERIFY(
>     -                NSK_CPP_STUB2(AddCapabilities, jvmti, &caps))) {
>     +                jvmti->AddCapabilities(&caps))) {
> 
>     --alex
> 
>     On 09/26/2018 20:37, JC Beyler wrote:
>      > Hi all,
>      >
>      > I continued the NSK_CPP_STUB removal with this webrev:
>      >
>      > Webrev: http://cr.openjdk.java.net/~jcbeyler/8211131/webrev.00/
>     <http://cr.openjdk.java.net/%7Ejcbeyler/8211131/webrev.00/>
>      > <http://cr.openjdk.java.net/%7Ejcbeyler/8211131/webrev.00/>
>      > Bug: https://bugs.openjdk.java.net/browse/JDK-8211131
>      >
>      > This does the first 50 files of the jvmti subfolder, it is done
>      > automatically by the scripts I put in the bug comments.
>      >
>      > Let me know what you think,
>      > Jc
> 
> 
> 
> -- 
> 
> Thanks,
> Jc


More information about the serviceability-dev mailing list