Kumar,
Thank you for your feedback; I will incorporate it in the next webrev.
-- Jon
On 04/25/2018 09:38 AM, Kumar Srinivasan wrote:
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
