RFR: 8212961: [TESTBUG] vmTestbase/nsk/stress/jni/ native code cleanup [v2]

Thomas Stuefe stuefe at openjdk.java.net
Fri Jul 23 05:41:04 UTC 2021


On Thu, 22 Jul 2021 23:11:36 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Please review what is primarily a code style cleanup with some old native code:
>> 
>> - add spaces around operators and after commas
>> - fixed indent and line breaks
>> - added braces to blocks
>> - replaced raw malloc with c_malloc (check malloc)
>> - fixed CE macro using and exception processing in general
>>   - added different CHECK macro for monitor enter/exit
>>   - removed some unreachable explicit exception checking code
>> 
>> I staged this as multiple commits to try and make the changes more obvious at each step (and if you hide whitespace changes it simplifies quite a bit), but it may be easier to just look at the end results and see if anything jumps out as still needing changing.
>> 
>> Note: there was no attempt to actually change the logic of any test or rewrite them in a better way - that is out of scope for the cleanup. The only logic changes relates to the CE macro and exception/error checking.
>> 
>> Testing:
>> - local running of the tests themselves
>> - tier5 testing in our CI system (in progress).
>> 
>> This may be of interest to @iignatev  and @lmesnik .
>> 
>> Thanks,
>> David
>
> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Restored 'dead code' that led to compilation failure on Windows

Looks good, small remarks inside.

Cheers, Thomas

test/hotspot/jtreg/vmTestbase/nsk/stress/jni/libjnistress001.cpp line 250:

> 248:   }
> 249:   CHECK(env->MonitorExit(jobj));
> 250:   return env->NewString(javachars->str[index - 1], elem_len);

Should you check for exceptions after NewString?

test/hotspot/jtreg/vmTestbase/nsk/stress/jni/libjnistress003.cpp line 89:

> 87:     } else {
> 88:       boolBuf[i] = JNI_FALSE;
> 89:     }

Shorten to `boolBuf[i] = (i % 2) ? JNI_TRUE : JNI_FALSE` ?

-------------

PR: https://git.openjdk.java.net/jdk/pull/4882


More information about the hotspot-runtime-dev mailing list