On Wed, 14 Feb 2024 11:41:43 GMT, Alan Bateman <[email protected]> wrote:
>> Please review this PR that makes the launcher helper keep a reference to the
>> executable JAR file active after extracting the name of the main class and
>> returning it as Class instance. Now, when loading classes from the JAR file,
>> it hasn't to be re-opened.
>
> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 596:
>
>> 594: String mainValue;
>> 595: try {
>> 596: Manifest manifest = jarFile.getManifest();
>
> I think the try-catch around the block can be dropped and you can put a more
> targeted try-catch around getManifest, at least I think that is the only case
> remaining in getMainClassFromJar that needs error handling now.
Dropping that outer try-catch will lead to many whitespace-only changes due to
un-indenting lines of the former block. Proceed or keep the indentation stable
by making that internal method throw IOE and insert a comment-only block like:
/* keep indentation stable */ {
Manifest manifest = jarFile.getManifest();
// ...
}
> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 876:
>
>> 874: jarFile.close();
>> 875: } catch (IOException ioe) {
>> 876: abort(ioe, "java.launcher.jar.error1", what);
>
> java.launcher.jar.error1 is "Error: An unexpected error occurred while trying
> to open file". I can't think of any cases where it might fail but the error
> message would be confusing if it did.
Good catch! Introducing and using:
java.launcher.jar.error5=\
Error: An unexpected error occurred while trying to close file {0}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17843#discussion_r1489697776
PR Review Comment: https://git.openjdk.org/jdk/pull/17843#discussion_r1489699548
