RFR: 7150256/8004095: Add back Remote Diagnostic Commands (original) (raw)

frederic parain frederic.parain at oracle.com
Thu May 2 18:27:23 UTC 2013

Karen,

Thank you for the review.

    1. 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