On Tue, 29 Jun 2021 23:19:43 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> Maxim Kartashev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Addressed PR comments >> >> 1. Allowed test to run on any platform. >> 2. Trimmed comments to fit in with 80 columns. >> 3. Removed unnecessayr comments. >> 4. Made the ExceptionDescribe() calls conditional on the value of >> FontUtilities.debugFonts() > > src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 146: > >> 144: freeNativeResources(env, scalerInfo); >> 145: (*env)->CallVoidMethod(env, scaler, invalidateScalerMID); >> 146: // NB: Exceptions must not be cleared (and therefore no JNI calls >> performed) after calling this method > > Please split long lines to 80 chars per line Done. > src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 199: > >> 197: bBuffer, offset, numBytes); >> 198: // This is a callback, we are not returning immediately to >> Java and better report exceptions now >> 199: CHECK_EXCEPTION(env); > > Probably we should report it only if "debugFonts" was set? Done. > test/jdk/java/awt/font/JNICheck/FreeTypeScalerJNICheck.java line 28: > >> 26: * @bug 8269223 >> 27: * @summary Verifies that -Xcheck:jni issues no warnings from >> freetypeScaler.c >> 28: * @requires os.family == "linux" > > Can we run this test on all platforms? Since this bug was not found, means we > did not cover this code by the tests, and it will be useful to test it even > if the code path will be different on other platforms. Sure; dropped the `@requires` clause. ------------- PR: https://git.openjdk.java.net/jdk/pull/4572
