Open code review for 8061999 Enhance VM option parsing to allow options to be specified

[Ron Durbin]

Dmitry,


Thanks for your time in reviewing the code and providing comments.
My responses to your comments inline below:


>-----Original Message-----
>From: Dmitry Dmitriev 
>Sent: Wednesday, June 24, 2015 3:26 AM
>To: Ron Durbin; hotspot-runtime-dev at openjdk.java.net
>Subject: Re: Open code review for 8061999 Enhance VM option parsing to allow options to be specified
>
>Hello Ron,
>
>Several comments for src/share/vm/runtime/arguments.cpp file:
>
>1) You can call os::close just after read on line 3945, because os::close is called in all execution paths.
>After that you can remove os::close on lines 3950, 3960, 3968 and 3975.
>


[Ron>] Great code cleanup. Will do

>2) Unneeded 'buf = NULL;' on lines 3949, 3959, 3967, 4026 and 4054.
>

[Ron>] While these particular cases are obvious that memory is free,
       I am following a consistent style that forces all code to address zero
       if a pointer is not set explicitly to an allocated memory block.

>3) Unneeded 'options_file = NULL;' on lines 4103, 4144, 4155, 4180, 4227, 4256 and 4276.
>

[Ron>] See previous reply


>4) Unneeded 'buff = NULL;' on lines 4178, 4225, 4254 and 4274
>

[Ron>] See previous reply

>5) You can call 'FreeVMOptions(&args, argsin);' just after parse_vm_init_args function call on line 4423,
>because FreeVMOptions is called in all execution paths. After that you can remove 'FreeVMOptions(&args,
>argsin);' on lines 4425 and 4429.
>

[Ron>] Great code cleanup. Will do.

>Thank you,
>Dmitry
>
>
>On 22.06.2015 15:52, Ron Durbin wrote:
>
>
>	Webrev URL:
>	http://cr.openjdk.java.net/~rdurbin/8061999_OCR0_JDK9_webrev
>
>
>	RFE request:
>	https://bugs.openjdk.java.net/browse/JDK-8061999
>
>	This RFE allows a file to be specified that holds VM Options that
>	would otherwise be specified on the command line or in an environment variable.
>	Only one options file may be specified on the command line and no options file
>	may be specified in either of the following environment variables
>	"JAVA_TOOL_OPTIONS" or "_JAVA_OPTIONS".
>
>	The options file feature supports all VM options currently supported on
>	the command line, except the options file option. The option to specify an
>	options file is "-XX:VMOptionsFile=<Filename>".
>	The options file feature supports an options file up to 1024 bytes in size
>	and up to 64 options.
>
>	This feature has been tested on:
>	  OS:
>	    Solaris, MAC, Windows, Linux
>	  Tests:
>	    Manual unit tests
>	    JPRT with -testset hotspot (including the SQE proposed test coverage for this feature.)
>
>
>