RFR: 8201274: Launch Single-File Source-Code Programs

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Wed Apr 25 16:38:32 UTC 2018


Hi John,

I focused mainly on the native side, looks ok, except  for a couple of
minor issues.

java.c

1320                 const char *prop = "-Djdk.internal.javac.source=";
1321                 size_t size = JLI_StrLen(prop) + JLI_StrLen(value) + 1;
1322                 char *propValue = (char *)JLI_MemAlloc(size + 1);

I think we are allocating extra byte ^^^^^^^

1323                 JLI_StrCpy(propValue, prop);
1324                 JLI_StrCat(propValue, value);

I think we can do this, safer and neater, as follows:

size_t size = JLI_StrLen(prop) + JLI_StrLen(value);
char *propValue = (char *)JLI_MemAlloc(size + 1);
JLI_Snprintf(propValue, size, "%s%s", prop, value);

1483     if (mode == LM_SOURCE) {
1484         AddOption("--add-modules=ALL-DEFAULT", NULL);
1485         *pwhat = SOURCE_LAUNCHER_MAIN_ENTRY;
1486         *pargc = argc + 1;
1487         *pargv = argv - 1;

A short comment perhaps ? why we are incrementing argc, and decrementing 
argv,
saves some head scratching for a casual reader.

I looked at the launcher tests, very nice.


Thanks
Kumar





On 4/12/2018 1:15 PM, Jonathan Gibbons wrote:
> Please review an initial implementation for the feature described in
> JEP 330: Launch Single-File Source-Code Programs.
>
> The work is described in the JEP and CSR, and falls into various parts:
>
>   * The part to handle the new command-line options is in the native
>     Java launcher code.
>   * The part to invoke the compiler and subsequently execute the code
>     found in the source file is in a new class in the jdk.compiler module.
>   * There are some minor Makefile changes, to add support for a new
>     resource file.
>
> There are no changes to javac itself.
>
> JEP: http://openjdk.java.net/jeps/330
> JBS: https://bugs.openjdk.java.net/browse/JDK-8201274
> CSR: https://bugs.openjdk.java.net/browse/JDK-8201275
> Webrev: http://cr.openjdk.java.net/~jjg/8201274/webrev.00/
>
> -- Jon




More information about the build-dev mailing list