RFR v4 - 8027634: Support @argfiles for java command-line tool

[Kumar Srinivasan]

Henry,

Generally looks good here are some comments, on my initial
pass, I am not fully done with args.c I will look at this some
more later today or tomorrow.

args.c:
1.) Can be folded

   45     char *rv;
   46
   47     rv = (char *) JLI_MemAlloc(len + 1);

char *rv = (char *) JLI_MemAlloc(len + 1);

2.) replace the defines with enum ?
  
   53 #define FIND_NEXT     0
   54 #define IN_COMMENT    1
   55 #define IN_QUOTE      2
   56 #define IN_ESCAPE     3
   57 #define SKIP_LEAD_WS  4
   58 #define IN_TOKEN      5

3.) I would rename the following, this is maintaining the state of
the state machine right ? So rename stage -> state

4.)
expectingNoDash is expectingMain right ? if so I would rename it so.

5.)

   84     if (isJava) {
   85         firstAppArgIndex = -1;
   86     } else {
   87         // tools, this value remains 0 all the time.
   88         firstAppArgIndex = 0;
   89     }
   90 }

can be written with unary operator

firstAppArgIndex = isJava ? -1 : 0;


TestHelper.java:

Why add findInOutput method ? this seems to be identical
to "matches" at line 606, which does exactly that.

ArgFileSyntax.java
Nice, but I think the creation and storage of test case can be simplified,
basically loadCases(), also please avoid  Xbootclasspath. The trouble
with these tests which operate loops, are very painful to debug
and isolate a failing case,  on the night before GA.

Can we do something like this.....

Map<String, List<String>> tests = new ArrayList<>();
tests.put("testing quotes", Array.asList({{...}, {....}});
tests.add("no recurse expansion", Array.asList({{...}, {....}});

The execute each test, with a description of what the test is doing.
This way if a test fails its easy to zero in on the failing test.

Kumar


On 8/8/2015 12:03 AM, Henry Jen wrote:
> Hi,
>
> Another update for argument file support, this version we added a couple measures to ensure compatibility,
>
> 1) Compile time directive ENABLE_ARG_FILES is needed to enable the feature, only java is enabled with this webrev.
> 2) Escape @argument if so desired: Additional prefixes of @ to a @ prefixed option will  act as an escape, ie. the first @ will be removed and the rest of the arguments presented to the launcher literally.
>   
> For example:
>   
> java @ foo  gets arguments of @ foo
> java @filename, filename expanded
> java @@filename will have the leading @ removed and reduced to literal @filename
> similarly,
> java @@@filenamewill have the leading @ removed and reduced to literal @@filename
>   
>   3) The option '-Xdisable- at files' can be used on the command line anywhere to prevent further @filename expansion.
>   4) Limit the file size must not exceed MAXINT (2, 147, 483, 647) bytes.
>
> Webrev athttp://cr.openjdk.java.net/~henryjen/jdk9/8027634/v4/webrev/
>
> Cheers,
> Henry
>