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

[Kurchi Hazra]

On 1/10/2012 4:58 PM, Stuart Marks wrote:
> 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.

This webrev includes the above changes List<Object> to List<?>

http://cr.openjdk.java.net/~khazra/7117570/webrev.05/

-Kurchi

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

-- 
-Kurchi