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.adams at oracle.com 
> <mailto:gary.adams at 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181220/bbf86453/attachment.html>