Review Request: Warnings cleanup in sun.management and its subpackages

Mandy Chung mandy.chung at oracle.com
Mon Dec 5 16:28:41 PST 2011


On 12/2/11 3:09 PM, Kurchi Hazra wrote:
> Hi,
>
> http://cr.openjdk.java.net/~khazra/7117570/webrev.00/
All these classes are implementation classes. These classes were
implemented before the 1.5 language features were integrated.
Below I included the suggestion to replace iterator with foreach
since you are already in that file.

Agent.java
    L221, 229: Since you're in that file, can you change it to
    call the UnsupportedOperationException(String msg, Throwable cause)
    constructor so that the cause exception is available?

    L252-271: it would be good to replace this try-finally with
    try-with-resource.

ConnectorAddressLink.java
    L120-128: it'd be better to change this to use foreach.
    L172-173: the variable c in the for-loop should be
       of type Counter.

GarbageCollectorImpl.java
    L76: another good candidate to use foreach.

HotspotCompilation.java
    L139-144: another good candidate to use foreach.

LazyCompositeData.java
    L162: for (String item : allItems) {...}

MappedMXBeanType.java
    L76, 88: the cast should be fixed to "(Class<>)"
    L229, 759: I think these suppressed warnings can be eliminated.
      I have to study it a little bit to determine what we can do
      with this file and get back to you.

NotificationEmitterSupport.java
    This class was copied fromjavax.management.NotificationBroadcasterSupport
    when it was implemented but the jmx class has been updated since then.
    It didn't use javax.management.NotificationBroadcasterSupport directly
    to avoid loading the jmx classes if java.lang.management is used locally
    with no jmx agent created.  I think it's time to update this class
    and I'll submit a CR for that.  Your change is fine.

ConnectorBootstrap.java
    L237: nit: this extra line which can be removed.

AdaptorBootstrap.java
    L130, 133: The com.sun.jmx.snmp API is not generified (it's internal API).
    Although the javadoc states that the getTrapDestinations method returns
    an enumeration of InetAddress, I think it'd be better for this change
    to wait until the com.sun.jmx.snmp API is updated.

JvmMemManagerTableMetaImpl.java
    L70: should be indented (looks like it was accidentially removed
    in your change).

    L122: the returned type should be List<MemoryManagerMXBean>, right?

    L107: JvmContextFactory.getUserData() returns a Map<Object,Object>
      which is the current implementation.  This strikes me that
      this may lead to some type casting somewhere.
      e.g. JvmMemoryImpl.java L160 - this casts the key to MemoryUsage.
      Shouldn't javac issue unchecked warning for this?  I wonder if
      you see any other warnings.

    I didn't review the rest of the snmp files (up to the JvmMemoryImpl.java
    file).  It takes longer that I expect.  It was very old code and
    looks like that it requires more investigation.  I suggest to separate
    the snmp change and first get the non-snmp files warning cleaned and
    continue with the snmp one next.

Thanks
Mandy




More information about the jdk8-dev mailing list