On Mon, 28 Feb 2022 18:44:27 GMT, Chris Plummer <[email protected]> wrote:
>> Alex Menkov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Chris's feedback
>
> test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002.java
> line 84:
>
>> 82: thread.start();
>> 83:
>> 84: // enable events requires live thread
>
> I think a more detailed comment could be used here. Something like:
>
> // setThread(thread) enables JVMTI events, and that can only be done on a
> live thread,
> // so wait until the thread has started.
Fixed.
> test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/hs201t002.cpp
> line 107:
>
>> 105: }
>> 106:
>> 107: if (!NSK_VERIFY(readNewBytecode(jvmti_env, newClassSize,
>> newClassBytes))) {
>
> For simple types I don't care for C++ pass-by-ref arguments. At the call site
> it just looks like the values are being passed in. When I first read this it
> didn't look right to me since they are uninitialized, so I had to look at
> readNewBytecode() to see what was going on.
Changed arguments to pointers
> test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t002/hs201t002.cpp
> line 368:
>
>> 366: if (threadName != NULL) {
>> 367: jvmti->Deallocate((unsigned char*)threadName);
>> 368: }
>
> callbackException() and callbackExceptionCatch() seem to be identical other
> than callbackException() taking a couple of extra unused arguments. They
> should probably both just call a shared function.
Fixed
-------------
PR: https://git.openjdk.java.net/jdk/pull/7607
