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