Hi Christoph,
We have some closed "attach on demand" tests that should be run also. I
can do this for you when you are ready. Please also let me know which
other jtreg tests you have run.
thanks,
Chris
On 4/9/18 12:08 AM, Langer, Christoph wrote:
Hi Chris,
thanks for looking into this.
As for ArgumentIterator::next, I must admit, I found this patch in our code base when
taking over the code. I believe that an issue would be seen if an attach operation has 2
or 3 arguments and the first one is NULL/empty. I guess such a situation can't happen
with the attach operations currently existing in OpenJDK as none of these ops would allow
such type of arguments. However, in our implementation, we have for instance enhanced the
"dump_heap" operation to work with null as first argument where one usually
would specify the desired output file name. We implemented a mechanism to compute a
default filename when the param is left blank. So we need the fix for that case, I guess.
I'll run the patch through the submission forest now and do some jtreg testing.
Best regards
Christoph
-----Original Message-----
From: Chris Plummer [mailto:chris.plum...@oracle.com]
Sent: Freitag, 6. April 2018 18:37
To: Langer, Christoph <christoph.lan...@sap.com>; serviceability-
d...@openjdk.java.net
Cc: hotspot-...@openjdk.java.net
Subject: Re: RFR (M): 8201247: Various cleanups in the attach framework
Hi Christoph,
Can you explain a bit more about "fix handling of null values in
ArgumentIterator::next". When does this turn up? Is there a test case?
Everything else looks good.
thanks,
Chris
On 4/6/18 8:01 AM, Langer, Christoph wrote:
Hi,
can I please get reviews for a set of clean up changes that I came
across when doing some integration work.
Bug: https://bugs.openjdk.java.net/browse/JDK-8201247
<https://bugs.openjdk.java.net/browse/JDK-8201247>
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8201247.0/
<http://cr.openjdk.java.net/%7Eclanger/webrevs/8201247.0/>
Detailed comments about the changes can be found in the bug.
Thanks & best regards
Christoph
