On Thu, 17 Oct 2024 06:14:21 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Jan Lahoda has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Adjusting the concise help as suggested: 'using main class of a JAR >> archive' and '<JarFile>.jar'/'<SourceFile>.java' > > src/java.base/share/native/libjli/java.c line 1935: > >> 1933: NULL_CHECK(printConciseUsageMessage = >> (*env)->GetStaticMethodID(env, cls, >> 1934: "printConciseUsageMessage", >> "(Z)V")); >> 1935: (*env)->CallStaticVoidMethod(env, cls, printConciseUsageMessage, >> printTo); > > Should we have a exception check here after the `CallStaticVoidMethod` > returns? I realize that several places in the current launcher code, in this > file, don't have those checks, but the JNI specific of `CallStaticVoidMethod` > does note that this function could raise any exception thrown by the > underlying Java method? I see `CHECK_EXCEPTION_LEAVE(1);` in the caller of this method. Given we could only do `CHECK_EXCEPTION_RETURN();`, which returns if an exception is thrown (so that the caller can handle the exception as they see fit), it seems unnecessary for last calls. As for last calls, it is "return if an exception is thrown, and return if it is not". Note it is not only this function, `PrintJavaVersion`, `ShowSettings`, `ShowResolvedModules`, `ListModules` and maybe other use the same pattern, where the caller handles the exceptions, and the functions don't check the exceptions for the last call. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21411#discussion_r1804183643
