On Tue, 21 Oct 2025 09:28:21 GMT, Francesco Andreuzzi <[email protected]> wrote:
>> 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. Usually, we use the test library functions `check_jvmti_error()` and `check_jvmti_status()` from the `test/lib/jdk/test/lib/jvmti/jvmti_common.hpp`. The `jvmti->GetErrorName()` can be used there but it is not. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27766#discussion_r2453414160
