RFR: JDK-8211343: nsk_jvmti_parseoptions should handle multiple suboptions
    Chris Plummer 
    chris.plummer at oracle.com
       
    Fri Dec 21 19:31:55 UTC 2018
    
    
  
Hi Gary,
This is much better. Just one question and a couple of minor comments:
The original code had the following:
  225     context.options.string = (char*)malloc(len + 2);
...
  231     strncpy(context.options.string, options, len);
  232     context.options.string[len] = '\0';
  233     context.options.string[len+1] = '\0';
Do you know why it was placing two NULLs at the end (and the first was 
unnecessary since strncpy already did that)?
  243             *value++ = '\0';
This should be rewritten in a manner that make it clearer what it does. 
At first glance you have to ask yourself whether it is setting *value or 
*(value++). Then hopefully you eventually clue in that they are both the 
same since "value++" evaluates to "value". I think the following would 
be better:
               *value = '\0';
               value++;
  235     /* Create a temporay copy of the options string to be 
tokenized. */
"temporay" typo.
thanks,
Chris
On 12/21/18 10:52 AM, Gary Adams wrote:
> Here is a first pass at a replacement parser for jvmti test options.
>
>   Webrev: http://cr.openjdk.java.net/~gadams/8211343/webrev.00/
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8211343
>
> Testing is in progress. Need to check out more platforms.
> On local linux testing one jvmti test failed SetNativeMethodPrefix001
> which passed an empty options string.
> ...
    
    
More information about the serviceability-dev
mailing list