|
On 8/28/17 12:41, Daniel D. Daugherty
wrote:
On 8/28/17 1:37 PM, serguei.spit...@oracle.com
wrote:
On 8/28/17 12:17, Daniel D.
Daugherty wrote:
On 8/22/17 5:22 PM, serguei.spit...@oracle.com
wrote:
Please, review another revision of the fix for the
enhancement:
https://bugs.openjdk.java.net/browse/JDK-8061228
CSR:
https://bugs.openjdk.java.net/browse/CCC-8061228
The SCR is in the DRAFT state.
Joe suggested to consider this CSR approved and gave a GO
for integration.
It will be moved to the right state later when the CSR
tools are ready.
I'm still asking at least one reviewer to look at this CSR
and give a thumbs up.
It is to ensure everything is going in a right direction.
Serguei,
I made minor editorial tweaks to the CSR and added myself as
a reviewer.
The CSR looks good to go.
Hi Dan,
Thank you a lot for the review including the CSR!
You seems to looked at 8061228-jdi.transport.2 that I
generated
temporarily for myself and which is obsolete now.
Okay... but the CSR referred to the ".2" webrev so that's why I
used it for my review... (and it's a bigger dot number)...
Oh, I see the problem.
At some point I decided to place the latest webrev to .1 as it has
not been published yet.
But I forgot about the link placed in the CSR.
Sorry for the confusion.
Let's keep the ".2" in CSR.
I'll place the update for you comments fixed in there.
Thanks,
Serguei
Dan
The 8061228-jdi.transport.1 was sent for review
and has to be used.
But I will consider and fix all the comments that are still
relevant for v1.
Thanks,
Serguei
Dan
I'll finalize the CSR after that.
Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8061228-jdi-transport.1/
The lastest webrev from Dmitry:
http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.18/
Incremental webrev vs the latest webrev from Dmitry:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8061228-jdi-transport.1.inc/
Summary:
This enhancement was developed by Dmitry who left the
team.
I don't know what email address to use to CC him at this
point.
I hope, Dmitry will find this discussion and reply
accordingly.
The latest webrev revision from Dmitry was v18 (please,
see above).
This revision covers the following:
- Cleanup for versioning negotiation protocol (back up
to the original).
Now the transport library supports both versions 1_0
and 1_1 (newly introduced).
- The transport native interface was changed.
The function SetTransportConfiguration() is introduced
instead of the
StartListeningWithAllow(). It allows to the same
transport library to support
both old and new version of the transport interface.
At this point, the
new structure jdwpTransportConfiguration includes only
one field:
const char* allowed_peers;
But it can be extended in the future if any
other update in configuration
will be required.
- The unit test was updated to provide better coverage
of the corner cases
for 'allow' option introduced by this enhancement.
- Fixes to improve diagnosability.
- A couple of bugs/regressions were fixed so that all
the JDI tests are passed now.
- A cleanup that includes some renaming and
reformatting.
Testing:
Tested new agent flag (allow), with new test:
jdk/test/com/sun/jdi/BasicJDWPConnectionTest.java
Ran the nsk.jdi, nsk.jdwp and jtreg jdk_jdi for both
release and fastdebug builds.
All tests are passed.
Thanks,
Serguei
|
