Code review request: 7088502 Security libraries don't build with javac -Werror

[Sean Mullan]

Hi Kurchi,

The updated webrev looks good.

--Sean

On 9/22/11 5:42 PM, Kurchi Hazra wrote:
> Hi Sean,
> 
> 
>      The updated webrev with your comments incorporated:
> http://cr.openjdk.java.net/~mullan/kurchi/7088502/webrev.01/
> 
> For
> SignatureAlgorithm.java
> 
> [130, 399, 434] change type to Class<? extends SignatureAlgorithmSpi>
> 
> 
> and similar changes, I have stuck to your comment, and suppressed the 
> "unchecked" warnings at the places I had to cast.
> 
> 
> Thanks,
> Kurchi
> 
> 
> 
> On 9/21/2011 7:58 AM, Sean Mullan wrote:
>> On 9/20/11 5:34 PM, Kurchi Hazra wrote:
>>
>>>> * MessageDigestAlgorithm.java
>>>>
>>>> [74-5] This is preferable:
>>>>
>>>> static ThreadLocal<Map<String, MessageDigest>>  instances=new
>>>>      ThreadLocal<HashMap<String, MessageDigest>>() {
>>>>      protected Map<String, MessageDigest>  initialize()...
>>> The above does not work since the compiler complains that if ThreadLocal
>>> is a HashMap, initialize() cannot override unless its return type is
>>> also HashMap. Even if I change it to
>>> static ThreadLocal<Map<String, MessageDigest>>  instances=new
>>>       ThreadLocal<HashMap<String, MessageDigest>>() {
>>>       protected HashMap<String, MessageDigest>  initialize()...
>>>
>>> the compiler throws an error as follows:
>>>
>>> ../../../../../../src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/MessageDigestAlgorithm.java:74:
>>> error: incompatible types
>>>      static ThreadLocal<Map<String, MessageDigest>>  instances=new
>>>                                                               ^
>>>     required: ThreadLocal<Map<String,MessageDigest>>
>>>     found:<anonymous ThreadLocal<HashMap<String,MessageDigest>>>
>> Try this instead?
>>
>>      static ThreadLocal<Map<String, MessageDigest>>  instances =
>>          new ThreadLocal<Map<String, MessageDigest>>() {
>>
>>          protected Map<String, MessageDigest>  initialValue() {
>>              return new HashMap<String, MessageDigest>();
>>          };
>>      };
>>
>>>> * Canonicalizer20010315.java
>>>>
>>>> [209,314] I'm curious about these changes. Instead of adding a new
>>>> method getSortedSetAsCollection and then suppressing the warnings for
>>>> that, it seems like it would be sufficient to just suppress the
>>>> warnings in this code, ex:
>>>>
>>>> @SuppressWarnings("unchecked")
>>>> ...
>>>>
>>>>   ns.getUnrenderedNodes(result);
>>>
>>> I did not put it inside the code, since the method has many lines of
>>> code and this would mean suppressing unchecked warnings generated
>>> anywhere in the method.
>> Ok, thanks for the explanation.
>>
>>>> * SignatureAlgorithm.java
>>>>
>>>> [130, 399, 434] change type to Class<? extends SignatureAlgorithmSpi>
>>> If I make this change (and similar changes in other classes), I need to
>>> cast at various places, and then I need to Suppress the unchecked
>>> warnings. Is this preferable?
>>> ../../../../../../src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/SignatureAlgorithm.java:412:
>>> warning: [unchecked] unchecked cast
>>>
>>> SignatureAlgorithm._algorithmHash.put(algorithmURI, (Class<? extends
>>> SignatureAlgorithmSpi>)Class.forName(implementingClass));
>>>                                                                                                                                      ^
>>>     required: Class<? extends SignatureAlgorithmSpi>
>>>     found:    Class<CAP#1>
>>>     where CAP#1 is a fresh type-variable:
>>>       CAP#1 extends Object from capture of ?
>>>
>>>
>>> Is there a workaround?
>> Not sure. If you can't figure out a workaround, then disregard my comment and
>> keep the current code.
>>
>> Thanks,
>> Sean
>