Re: RFR (M) 8210665: Clean up JNI_ENV_ARG and factorize the macros for vmTestbase/jvmti[R-U] tests

Wed, 12 Sep 2018 12:11:33 -0700

Hi JC,

Just a couple of minor suggestions in ji05t001.cpp:

 174     NSK_DISPLAY1("\nagent %s initializer: obtaining the JVMTI env ...\n", (indx==0)?"A":"B");

I think I like this better as two lines.

 204     if ((err = jvmti[indx]->SetEventNotificationMode(JVMTI_ENABLE,
 205             JVMTI_EVENT_VM_INIT, NULL)) != JVMTI_ERROR_NONE) { /* enable event globally */

If I had my way you'd be making a couple dozen changes to remove assignments from conditional expressions, but I've been shot down when suggesting this in the past because doing this is considered within the coding guidelines. However, when the conditional takes up two lines, it really does start to become too hard to read. I'd suggest assigning "err" before the "if".

thanks,

Chris

On 9/12/18 11:45 AM, JC Beyler wrote:
Hi all,

I am continuing the clean up of the testbase with the next batch, I know this is getting repetitive but bear with me please, after this webrev, we have in vmTestbase:

- 29 more files that have the JNI_ENV* or JVMTI_ENV* macros (for a subsequent webrev)
- 400+ files that have trivial #ifdef __cplusplus macros around the extern "C" and the final } (for a second subsequent webrev)

After those two webrev, we can go to doing more important refactoring to get the vmTestbase in shape to start migrating out of there hopefully.

So, without further ado, here is another one with 50 file changes and 1283 line changes.


This passes testing for the changed tests on my dev machine.

Thanks for your reviews,
Jc


Reply via email to