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