Code Review Request: 7117570: Warnings in sun.mangement.* and its subpackages

[Stuart Marks]

On 1/10/12 4:35 PM, Kurchi Hazra wrote:
> Updated webrev with all the above changes incorporated:
> http://cr.openjdk.java.net/~khazra/7117570/webrev.04/
>
>> SnmpNamedListTableCache.java
>> L216,221,225: should it be List<?> rather than List<Object>?
>> Will that help get rid of the unchecked suppressed warning?
> - It does remove the suppressed warning added to this function, but will
> give rise to added suppress warnings in other places. For example:
>
> src/share/classes/sun/management/snmp/util/SnmpListTableCache.java:109
> (since now the argument passed to be has to be List<?>, else the compiler
> complains)
>
> Do you still want me to change it to List<?> and not List<Object>?

I was looking at this too. Arguably the best conversion of a raw type List to 
generics is List<?> but as you noted this causes a ripple effect, where other 
things have to be converted to wildcards as well. It looks like you have to 
change updateCachedDatas() and SnmpListTableCache.updateCachedDatas(), but then 
that's it. That doesn't look too bad. It could be that I've missed something 
and that you'll have to change more and more, in which case I'd reconsider 
making the change to use wildcards.

>> You mentioned in your previous email that sun.management and its
>> subpackages are warning free with your changeset but I suspect
>> there are still warnings e.g.
>> JvmMemoryImpl.java L160 - this casts the key to MemoryUsage.
> This and I also see some other casts that are somehow not being
> reported even if I turn on -Werror in make/sun/management/snmp. I will
> let this be for the time being and first get this changeset pushed. Probably
> in a new CR I will try adding -Werror to make/java/management and
> make/sun/management.

This cast doesn't generate an unchecked warning, because it *is* checked -- the 
VM checks the downcast from Object to MemoryUsage and throws ClassCastException 
if there's a mismatch. The compiler only emits unchecked warnings when the cast 
can't be checked at compile time *and* it can't be checked at runtime.

Offhand I don't know whether this code is correct in making this cast. (The 
cast is *checked* but it might not be *safe*.) The code does catch 
RuntimeException and it does something reasonable, so maybe it's OK. This 
effort is about cleaning up warnings, though, and since there's no warning here 
I don't think we need to worry about it.

s'marks


>
>
> Thanks,
> Kurchi
>
>
>>
>> Did you get a chance to check the incremental build and see if
>> there are warnings or not? e.g. cd sun/management; make clean all
>> I suspect the snmp code still has compiler warnings but that's fine
>> since it's very old code that requires more cleanup work for the
>> future.
>>
>> Mandy
>>
>> On 1/9/2012 12:02 PM, Kurchi Hazra wrote:
>>> Hi,
>>>
>>> As an effort to cleanup build warnings, this webrev involves small changes in
>>> sun.management.* and its subpackages:
>>>
>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7117570
>>> Webrev: http://cr.openjdk.java.net/~khazra/7117570/webrev.03/
>>>
>>>
>>> Thanks,
>>> Kurchi
>