JDK-8211343: nsk_jvmti_parseoptions should handle multiple suboptions

[JC Beyler]

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> 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/0d2f13f4/attachment-0001.html>