RFR: JDK-8317635: Improve GetClassFields test to verify correctness of field order [v2]
Serguei Spitsyn
sspitsyn at openjdk.org
Thu Oct 19 00:58:47 UTC 2023
On Wed, 18 Oct 2023 01:40:18 GMT, Alex Menkov <amenkov at openjdk.org> wrote:
>> All test cases in getclfld007 had 1 (or 0) field in test classes/interfaces.
>> The change adds several fields in one of the test classes to verify order of the returned fields (as described by GetClassFields spec: "in the order they occur in the class file").
>> Field order in the class file is not guaranteed to be the same as in the source, so information about expected fields and expected order is extracted by ASM (it parses class file sequentially).
>> This allows to drop hardcoded field name/type in native part.
>>
>> Additionally did some test cleanup:
>> - dropped "printdump" stuff (the test always logs reported fields);
>> - removed unused `generic` in native check() method, added deallocation of `name` and `sig`
>
> Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:
>
> get field order from class file
test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassFields/getclfld007/getclfld007.cpp line 133:
> 131: }
> 132: }
> 133: }
Nit: I'd suggest to refactor a little bit the content of loop at L111 as below:
for (j = 0; j < fcount; j++) {
if (fields[j] == NULL) {
printf("(%d) fieldID = null\n", j);
result = STATUS_FAILED;
continue; => /you can consider to return here
}
err = jvmti->GetFieldName(clazz, fields[j], &name, &sig, NULL);
if (err != JVMTI_ERROR_NONE) {
printf("(GetFieldName#%d) unexpected error: %s (%d)\n",
j, TranslateError(err), err);
result = STATUS_FAILED; => This result set is missed
continue; => you can consider to return here
}
printf(">>> [%d]: %s, sig = "%s"\n", j, name, sig);
if ((j < field_count) &&
(name == NULL || sig == NULL ||
!equals_str(env, name, fieldArr, j * 2) ||
!equals_str(env, sig, fieldArr, j * 2 + 1))) {
printf("(%d) wrong field: "%s%s"", j, name, sig);
result = STATUS_FAILED;
}
jvmti->Deallocate((unsigned char *)name);
jvmti->Deallocate((unsigned char *)sig);
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16131#discussion_r1364749665
More information about the serviceability-dev
mailing list