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

Xuelei Fan Xuelei.Fan at Oracle.Com
Fri Sep 23 00:00:21 PDT 2011


I only reviewed my commented changes. If you updated other files, please let me know.

Just one minor comment, read inlines, please.

On Sep 23, 2011, at 2:36 PM, Kurchi Hazra <kurchi.subhra.hazra at oracle.com> wrote:

> Hi Xuelei,
> 
>   Thanks a lot for your comments. Please find an updated webrev at :
> http://cr.openjdk.java.net/~xuelei/7092375/webrev.01/
> 
> 
> Thanks,
> Kurchi
> 
> 
> 
> On 9/21/2011 3:44 AM, Xuelei Fan wrote:
>> 1. "src/share/classes/javax/security/auth/SubjectDomainCombiner.java":
>>     public Void run() {
>>         // Call refresh only caching is disallowed
>> -           javax.security.auth.Policy.getPolicy().refresh();
>> +           refreshPolicy();
>>             return null;
>>    }
>> 
>> Personally, in general, I would not add a new method in order to cleanup
>> the warning.
>> 
>> What do you think if we suppress the warning at run() method?
>> +   @SuppressWarnings("deprecation")
>>     public Void run() {
>>         // Call refresh only caching is disallowed
>>            javax.security.auth.Policy.getPolicy().refresh();
>>             return null;
>>    }
>> 
>> 
>> 2. src/share/classes/javax/security/auth/x500/X500Principal.java
>> +    @SuppressWarnings("unchecked")
>>      public X500Principal(String name) {
>> -        this(name, (Map<String, String>) Collections.EMPTY_MAP);
>> +        this(name,(Map<String, String>)(Map) Collections.emptyMap());
>>      }
>> 
>> 
>> Personally, I would prefer the following update:
>>      public X500Principal(String name) {
>> -        this(name, (Map<String, String>) Collections.EMPTY_MAP);
>> +        this(name, Collections.<String, String>emptyMap());
>>      }

I don't think we need the suppress warnings annotation on this method any more.

Otherwise, look fine to me.

Xuelei


>> 
>> 3. src/share/classes/sun/security/ec/ECPublicKeyImpl.java
>> 
>>      protected void parseKeyBits() throws InvalidKeyException {
>>          try {
>>              AlgorithmParameters algParams = this.algid.getParameters();
>>              params = algParams.getParameterSpec(ECParameterSpec.class);
>> -            w = ECParameters.decodePoint(key, params.getCurve());
>> +            w = getDecodePoint(key, params.getCurve());
>>          } catch (IOException e) {
>>              throw new InvalidKeyException("Invalid EC key", e);
>>          } catch (InvalidParameterSpecException e) {
>>              throw new InvalidKeyException("Invalid EC key", e);
>>          }
>>      }
>> 
>> As #1, in general, I would not add a new method in order to cleanup the
>> warning.
>> 
>> What do you think if we suppress warnings in the method parseKeyBits()
>> level?
>> +    @SuppressWarnings("deprecation")
>>      protected void parseKeyBits() throws InvalidKeyException {
>> 
>> 
>> Otherwise, looks fine to me.
>> 
>> Thanks for the cleanup.
>> 
>> Xuelei
>> 
>> On 9/20/2011 11:27 PM, Kurchi Hazra wrote:
>>> Hi Xuelei,
>>> 
>>> Can you please review these changes?
>>> 
>>> Summary:
>>> 
>>> 1. Small changes to Java files, mostly in
>>> src/share/classes/javax/security and its subpackages to remove build
>>> warnings.
>>> 
>>> 2. Small changes to relevant makefiles to prevent
>>> reintroduction of removed warnings. Added a new makefile to prevent
>>> reintroduction of warnings in javax/security.
>>> 
>>> 
>>> webrev:http://cr.openjdk.java.net/~xuelei/7092375/webrev.00/
>>> 
>>> Bug description: To appear on http://bugs.sun.com/bugdatabase
>>> 
>>> 
>>> Thanks,
>>> Kurchi
>>> 
>>> 
> 
> -- 
> -Kurchi
> 



More information about the security-dev mailing list