Karen,
Thank you for the review.
1. 2. and 3. have been fixed.
Regarding 4.
Most (all?) permissions have a name argument.
However, if p._name is null, the string "(null)" is printed.
This is not wrong, but not very pretty:
Permission: MyPermission((null)).
I've changed the code (both 105 and 109) with a short conditional:
... p._name == NULL ? "null" : p._name ...
which gives a better output:
Permission: MyPermission(null)
Fred
On 02/05/2013 18:37, Karen Kinnear wrote:
Frederic,
Code looks good - actually it looks very clean. Ship it.
Couple of minor comments that don't require re-review:
1. nmtDCmd.hpp/cpp - copyrights 2012 -> 2012, 2013
2. jmm.h
line 213: "True is" -> "True if"
3. diagnosticFramework.hpp
Thank you for the comments!
line 298 "rational" -> "rationale"
4. diagnosticCommand.cpp
lines 105/109 - what prints if p._name is null?
thanks,
Karen
On Apr 30, 2013, at 12:26 PM, frederic parain wrote:
Hi all,
This is a second request for review to add back
Remote Diagnostic Commands.
This work adds a new platform MBean providing
remote access to the diagnostic command framework
via JMX (already accessible locally with the jcmd
tool).
There's two CR number because this work is made of two
parts pushed to two different repositories.
JDK changeset CR 7150256
http://cr.openjdk.java.net/~fparain/7150256/webrev.06/
HotSpot changeset: CR 8004095
http://cr.openjdk.java.net/~fparain/8004095/webrev.06/
Questions from previous review have been answered
in initial review threads. Changesets also include
some minor changes coming from internal audit and
feedback sent in private e-mails.
However, one issue is still pending: some unit tests
use a hard coded port number, which could cause test
failures if several instances of the same test are
run on the same machine. I propose to postpone the
fix of this issue after the JDK8 feature freeze
(leaving for vacations soon, I won't have time to
fix tests before the feature freeze).
Thanks,
Fred
--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: frederic.par...@oracle.com
--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: frederic.par...@oracle.com
