> On Jan 24, 2017, at 12:41 PM, Mandy Chung <mandy.ch...@oracle.com> wrote: > > >> On Jan 24, 2017, at 10:20 AM, Henry Jen <henry....@oracle.com> wrote: >> >> Hi, >> >> Please review the webrev[1] that add support for JAVA_OPTIONS environment >> variable. The bug[2] describes how JAVA_OPTIONS works. >> >> [1] http://cr.openjdk.java.net/~henryjen/jdk9/8170832/4/webrev/ > > This looks quite good. A couple of minor comments: > > 503 // Must be after expansion so we can caught if main class > specified @argfile > > typo: s/caught/catch. It’d be clear to simply say: > // Check if Main class specified after argument checked. > // This check must be done after expansion. >
Fixed. > 42 #define JAVA_OPTIONS “JAVA_OPTIONS" > > I think java.h is the appropriate file to declare this instead of jli_util.h. > Well, no disagreement. I moved it and add a separate entry in args.c. The reason it was in jli_util.h is follows the _JAVA_LAUNCHER_DEBUG. and JAVA_OPTIONS is for launcher. It is used by args.c which include jli_util.h but not full java.h in standalone mode. > emessages.h > Would it be clearer to rename ARG_INFO to ARG_INFO_ENVVAR? > Again, try o be consistent with existing style. Changed. Cheers, Henry
