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