On Fri, 3 Nov 2023 14:26:29 GMT, Vicente Romero <vrom...@openjdk.org> wrote:
>> Jim Laskey has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 12 additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'upstream/master' into 8315458 >> - Don't get args unless necessary >> - Remove unnamed classes from examples.not-yet.txt >> - Requested corrections >> - Changes recommended by Jan >> - Revised implicit class test >> - Don't store main method info globally. Use addition calls to fetch info. >> - Update JEP number in PreviewFeature >> - Remove MANDATED flag from implicit classes >> - Remove .orig files >> - ... and 2 more: https://git.openjdk.org/jdk/compare/64f1c416...0bd5b477 > > src/java.base/share/native/libjli/java.c line 559: > >> 557: >> 558: /* Build platform specific argument array */ >> 559: if ((mainType & MAIN_WITHOUT_ARGS) == 0) { > > nice, thanks, a minor addition to the condition: `&& argc > 0`? Need an empty array when argc == 0. > src/java.base/share/native/libjli/java.c line 561: > >> 559: if ((mainType & MAIN_WITHOUT_ARGS) == 0) { >> 560: mainArgs = CreateApplicationArgs(env, argv, argc); >> 561: CHECK_EXCEPTION_NULL_LEAVE(mainArgs); > > side: this is just a comment not proposing any change, I know it is a good > practice to check for nulls here, but having a null here is really possible? > how can we have a null here? but I guess this also check for any other > exception right? Yes it's a combo thing. This was the code that was there BTW. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16461#discussion_r1381823409 PR Review Comment: https://git.openjdk.org/jdk/pull/16461#discussion_r1381824589