[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v2]
From:       Maxim Kartashev <github.com+28651297+mkartashev () openjdk ! java ! net>
Date:       2021-06-30 10:42:52
Message-ID: lx0KD6GQ8tkpHDM055M1UUKtTDRwNbYYL1DJ5vWrD_g=.dcaa6acd-f795-46cd-81d8-b9f1ca87beab () github ! com
[Download RAW message or body]

On Tue, 29 Jun 2021 23:19:43 GMT, Sergey Bylokhov <serb@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


[prev in list] [next in list] [prev in thread] [next in thread]