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 >
