[OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v2]

[Maxim Kartashev]

On Wed, 30 Jun 2021 00:08:22 GMT, Phil Race <prr at 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 53:
> 
>> 51:         (*(env))->ExceptionDescribe(env);\
>> 52:         (*(env))->ExceptionClear(env);   \
>> 53:     }
> 
> https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#exceptiondescribe
> 
> "The pending exception is cleared as a side-effect of calling this function"
> 
> So you certainly don't need both of these and I would prefer that Describe only be used if really debugging where we think there's a REAL chance of an exception rather than just to keep JNI happy.
> 
> And the upcall that is likely (readBlock) itself will log any IOException (and catch it) in the event of an I/O error so I really think this is unlikely to be useful here.

Yes, makes sense. I made `CHECK_EXCEPTION()` call either `Clear...` or `Describe...` based on the value of `debugFonts`. 
Please, take a look.

> src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 198:
> 
>> 196:                                           sunFontIDs.ttReadBlockMID,
>> 197:                                           bBuffer, offset, numBytes);
>> 198:             // This is a callback, we are not returning immediately to Java and better report exceptions now
> 
> I think the comment is un-needed .. since the only reason to call CHECK_EXCEPTION() is because of this.
> Same for the other case.

OK, comments removed.

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

PR: https://git.openjdk.java.net/jdk/pull/4572