On Wed, 30 Jun 2021 00:08:22 GMT, Phil Race <p...@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
