RFR: 7150256/8004095: Add back Remote Diagnostic Commands
frederic parain
frederic.parain at oracle.com
Thu May 2 18:27:23 UTC 2013
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.Parain at oracle.com
>
--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: Frederic.Parain at oracle.com
More information about the core-libs-dev
mailing list