RFR (M) round 3 8061999 Enhance VM option parsing to allow options to be specified

[Daniel D. Daugherty]

Greetings,

I'm handling the proxy push for Ron's fix.

For the record, here are the minor changes made after
the round 3 code review:

$ diff -c -w src/share/vm/runtime/arguments.cpp{.ocr3,}
*** src/share/vm/runtime/arguments.cpp.ocr3     Thu Sep  3 21:31:21 2015
--- src/share/vm/runtime/arguments.cpp  Fri Sep  4 12:50:06 2015
***************
*** 3521,3528 ****
     // '+ 1' for NULL termination even with max bytes
     int bytes_alloc = OPTION_BUFFER_SIZE + 1;

!   char *buf = NEW_C_HEAP_ARRAY(char, bytes_alloc, mtInternal);
!
     if (NULL == buf) {
       jio_fprintf(defaultStream::error_stream(),
                   "Could not allocate read buffer for options file 
parse\n");
--- 3521,3527 ----
     // '+ 1' for NULL termination even with max bytes
     int bytes_alloc = OPTION_BUFFER_SIZE + 1;

!   char *buf = NEW_C_HEAP_ARRAY_RETURN_NULL(char, bytes_alloc, mtInternal);
     if (NULL == buf) {
       jio_fprintf(defaultStream::error_stream(),
                   "Could not allocate read buffer for options file 
parse\n");
***************
*** 3541,3547 ****
     os::close(fd);
     if (bytes_read < 0) {
       FREE_C_HEAP_ARRAY(char, buf);
-     buf = NULL;
       jio_fprintf(defaultStream::error_stream(),
                   "Could not read options file '%s'\n", file_name);
       return JNI_ERR;
--- 3540,3545 ----
***************
*** 3550,3556 ****
     if (bytes_read == 0) {
      // tell caller there is no option data and that is ok
      FREE_C_HEAP_ARRAY(char, buf);
-    buf = NULL;
      return JNI_OK;
     }

--- 3548,3553 ----
***************
*** 3557,3563 ****
     // file is larger than OPTION_BUFFER_SIZE
     if (bytes_read > bytes_alloc - 1) {
       FREE_C_HEAP_ARRAY(char, buf);
-     buf = NULL;
       jio_fprintf(defaultStream::error_stream(),
                   "Options file '%s' is larger than %d bytes.\n",
                   file_name, bytes_alloc - 1);
--- 3554,3559 ----
***************
*** 3880,3885 ****
--- 3876,3883 ----
     ScopedVMInitArgs java_tool_options_args;
     ScopedVMInitArgs java_options_args;
     ScopedVMInitArgs modified_cmd_line_args;
+   // Pass in vm_options_file_args to keep memory for flags_file from being
+   // deallocated if found in the vm options file.
     ScopedVMInitArgs vm_options_file_args;

     jint code =

Dan


On 9/4/15 2:29 PM, Ron Durbin wrote:
> David,
>
>   Thanks for your review, replies inline.
>
>
>> -----Original Message-----
>> From: David Holmes
>> Sent: Thursday, September 03, 2015 10:16 PM
>> To: Ron Durbin; hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: RFR (M) round 3 8061999 Enhance VM option parsing to allow options to be specified
>>
>> Hi Ron,
>>
>> On 4/09/2015 8:34 AM, Ron Durbin wrote:
>>> Here is the round 3 webrev for 8061999.
>>>
>>> Here are the links to round 2 and 3 patch files.
>>>
>>> Round 2: : http://cr.openjdk.java.net/~rdurbin/8061999_OCR2_JDK9_webrev/hotspot.patch
>>> Round 3: : http://cr.openjdk.java.net/~rdurbin/8061999_OCR3_JDK9_webrev/hotspot.patch
>>>
>>> Diff the patch files to see what changed in round 3.
>>>
>>>
>>> Webrev link: http://cr.openjdk.java.net/~rdurbin/8061999_OCR3_JDK9_webrev
>> 3512   char *buf = NEW_C_HEAP_ARRAY(char, bytes_alloc, mtInternal);
>> 3513
>> 3514   if (NULL == buf) {
>>
> [Ron>] Changed allocator from NEW_C HEAP_ARRAY to NEW_C_HEAP_ARRAY_RETURN_NULL to allow local error handing.
>
>> You will never get NULL as vm_exit_out_of_memory will be invoked if the
>> underlying AllocateHeap fails. (IIRC someone pointed that out in a
>> previous round.)
>>
>> 3532     buf = NULL;
>>
> [Ron>] Removed
>> Style nit: I don't see any point in NULL'ing a local when you are about
>> to rerturn.
>>
>> Otherwise seems ok.
>>
>> Thanks,
>> David
>>

On 9/4/15 2:34 PM, Ron Durbin wrote:
 > Coleen,
 >
 > Thanks for the review, replies inline.
 >
 >> -----Original Message-----
 >> From: Coleen Phillimore
 >> Sent: Thursday, September 03, 2015 6:37 PM
 >> To: hotspot-runtime-dev at openjdk.java.net
 >> Subject: Re: RFR (M) round 3 8061999 Enhance VM option parsing to 
allow options to be specified
 >>
 >>
 >> Ron,
 >>
 >> This looks good.  Could you put a short comment at line, ie:
 >>
 >> // Pass in vm_options_file_args to keep memory for flags_file from being
 >> deallocated if found in the vm options file.
 >>
 > [Ron>] comment added
 >> 3871   ScopedVMInitArgs vm_options_file_args;
 >>
 >>
 >> Or something like that?  In case someone (formerly, me) thinks it should
 >> be in a more local scope.
 >>
 >> Thanks,
 >> Coleen