On Tue, 21 Oct 2025 07:08:10 GMT, Serguei Spitsyn <[email protected]> wrote:

>> Francesco Andreuzzi has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   fix tool call
>
> test/hotspot/jtreg/serviceability/attach/EarlyDynamicLoad/libEarlyDynamicLoad.cpp
>  line 47:
> 
>> 45: 
>> 46:     jvmti->SetEventCallbacks(&callbacks, sizeof(callbacks));
>> 47:     jvmti->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_VM_START, 
>> nullptr);
> 
> Nit: Even though it is normally works well it'd be better to check and handle 
> the `jvmtiError` code returned from the JVMTI functions. You can easily find 
> some examples to follow. Also, the indent for native code has to be 2, not 4. 
>  It is possible, you can find some tests where this kind of check/handling is 
> missed or the indent is incorrect. It does not mean we should have these bad 
> habits with new tests. :)

Fixed in a401f0a118785f600e70ca0099802ff05e967bc9 and 
925f9fc828597f920e4b800d63428e414f8b0345. I thought the error could be printed 
with `jvmti->GetErrorName`, but I guess that would introduce unnecessary 
complication. Let me know what you think @sspitsyn.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27766#discussion_r2447475060

Reply via email to