Re: JDK-8211343: nsk_jvmti_parseoptions should handle multiple suboptions

[Gary Adams]

I prototyped with strsep, because strtok is not safe
and did not want to deal with the strtok_r versus
strtok_s (windows) platform variants.

...
#include <stdio.h>
#include <string.h>

int  main(int argc, char **argv){
  char *str = strdup("waittime=5,verbose foo bar baz=11");
  char *name;
  char *value;

  while ((name = strsep (&str, " ,")) != NULL) {
    value = index(name, '=');

    if (value == NULL) {
      printf ("%s\n", name);
    } else {
      *value++ = '\0';
      printf ("%s=%s\n", name, value);
    }
  }

...

./main
waittime=5
verbose
foo
bar
baz=11


On 12/20/18, 10:54 AM, JC Beyler wrote:

Hi Gary,

I had seen that too and forgot to file it! So thanks!

I think the comment you removed is interesting, I'm not sure what it means exactly and I've tried re-reading a few times but the use of "in this question" is weird :-)

Anyway, the webrev looks good except perhaps for the use of strsep, which is BSD, instead of strtok, which is C89/C99/Posix.

Thanks for doing this!
Jc

On Thu, Dec 20, 2018 at 5:17 AM Gary Adams <gary.ad...@oracle.com <mailto:gary.ad...@oracle.com>> wrote:

    During some earlier jvmti test debugging, I noticed that it was not
    possible to
    add a quick argument to the current tests and rerun the test. e.g.
    expand "waittime=5" to
    "waittime=5,verbose". I tracked down the options parsing in
    jvmti_tools.cpp and saw some
    comments that only a single option could be parsed.

    So I filed this bug to revisit the issue for the next release:

       Issue: https://bugs.openjdk.java.net/browse/JDK-8211343

    I think the option parsing should be ripped out and redone,
    but for now I think a simple tweak to use strsep(), might go a
    long way
    to solving the multiple option support. I just started testing,
    but wanted to know if anyone else has been down this rabbit hole
    before.


    diff --git
    a/test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp
    b/test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp
    --- a/test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp
    +++ b/test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp
    @@ -196,22 +196,14 @@
      }


    -/**
    - *
    - * The current option will not perform more than one
    - * single option which given, this is due to places explained
    - * in this question.
    - *
    - **/
    -
       /*
    -  * This whole play can be reduced with simple StringTokenizer
    (strtok).
    -  *
    +   * Parse a comma or space separated list of options.
        */

      int nsk_jvmti_parseOptions(const char options[]) {
          size_t len;
          const char* opt;
    +    char *str = strdup(options);
          int success = NSK_TRUE;

          context.options.string = NULL;
    @@ -232,7 +224,7 @@
          context.options.string[len] = '\0';
          context.options.string[len+1] = '\0';

    -    for (opt = context.options.string; ;) {
    +    while ((opt = strsep(&str, " ,")) != NULL) {
              const char* opt_end;
              const char* val_sep;
              int opt_len=0;



--

Thanks,
Jc