Code review request: 7064075 Security libraries don't build with javac -Xlint:all, -deprecation -Werror

Alexandre Boulgakov alexandre.boulgakov at oracle.com
Tue Jul 26 10:31:49 PDT 2011


I just noticed that pkcs11 is not built on my machine (64-bit Windows) 
so I missed all of the warnings there. There are two mission 
serialVersionUID warnings for classes that have had different generated 
serialVersionUID's in the past.

sun.security.pkcs11.P11Key.P11SecretKey
-currently: -7828241727014329084L;
-JDK 1.5: -897881148551545872L;

sun.security.pkcs11.P11TlsPrfGenerator$1
-currently: -8090049519656411362L;
-JDK 6: -3305145912345854189L;

I'm not sure why the serialVersionUID changed for 
sun.security.pkcs11.P11TlsPrfGenerator$1; the code is the same, and the 
serialVersionUID for the base class javax.crypto.SecretKey hasn't changed.

For sun.security.pkcs11.P11Key.P11SecretKey, the code is the same, but 
the base class sun.security.pkcs11.P11Key has changed.

How should I go about resolving these issues?

Thanks,
Sasha

On 7/20/2011 3:37 PM, Xuelei.Fan at Oracle.Com wrote:
>
> On Jul 21, 2011, at 1:25 AM, Alexandre Boulgakov<alexandre.boulgakov at oracle.com>  wrote:
>
>> This is a Netbeans warning, not generated by the compiler. The reason is that List.isEmpty() can be more efficient for some implementations. ArrayList.size() == 0 and ArrayList.isEmpty() should take the same time, so it doesn't matter for allResults, but keyTypeList is a List argument, so any implementation could be passed in. List.isEmpty() should never be slower than List.size() == 0 because AbstractCollection defines isEmpty() as size() == 0.
>>
>> Even if we don't get a performance improvement, it still improves readability.
>>
> Sounds reasonable.
>
> Thanks,
> Xuelei
>
>> -Sasha
>>
>> On 7/19/2011 7:32 PM, Xuelei Fan wrote:
>>> I was looking at the updates in sun/security/ssl.  Looks fine to me.
>>>
>>> It's fine, but I just wonder why List.isEmpty() is preferred to
>>> (List.size() == 0). What's the compiler warning for (List.size() == 0)?
>>>
>>> src/share/classes/sun/security/ssl/X509KeyManagerImpl.java
>>> -        if (keyTypeList == null || keyTypeList.size() == 0) {
>>> +        if (keyTypeList == null || keyTypeList.isEmpty()) {
>>>
>>> -        if (allResults == null || allResults.size() == 0) {
>>> +        if (allResults == null || allResults.isEmpty()) {
>>>
>>> Thanks for the cleanup.
>>>
>>> Thanks,
>>> Xuelei (Andrew) Fan
>>>
>>> On 7/20/2011 7:22 AM, Alexandre Boulgakov wrote:
>>>> Hello Sean,
>>>>
>>>> Have you had a chance to look at this webrev?
>>>>
>>>> Thanks,
>>>> Sasha
>>>>
>>>> On 7/18/2011 6:21 PM, Alexandre Boulgakov wrote:
>>>>> Hello,
>>>>>
>>>>> Here's an updated webrev:
>>>>> http://cr.openjdk.java.net/~smarks/aboulgak/7064075.2/
>>>>>
>>>>> I've reexamined the @SuppressWarnings("unchecked") annotations, and
>>>>> added comments to all of the ones I've added. I was was also able to
>>>>> remove several of them by using covariant return types on
>>>>> sun.security.x509.*Extension.get(String) inherited from Object
>>>>> CertAttrSet<T>.get(String). I've also updated the consumers of
>>>>> sun.security.x509.*Extension.get(String) to use the more specific
>>>>> return type, removing several casts and @SuppressWarnings("unchecked")
>>>>> annotations.
>>>>>
>>>>> Also, please take a closer look at my changes to
>>>>> com.sun.security.auth.PolicyFile.getPrincipalInfo(PolicyParser.PrincipalEntry,
>>>>> final CodeSource) in
>>>>> src/share/classes/com/sun/security/auth/PolicyFile.java lines
>>>>> 1088-1094. The preceding comment and the behavior of
>>>>> Subject.getPrincipals(Class<T>) seem to be more consistent with the
>>>>> updated version, but I wanted to make sure.
>>>>>
>>>>> The classes where I added serialVersionUID's are either new or have
>>>>> the same serialVersionUID as the original implementation.
>>>>>
>>>>> Thanks,
>>>>> Sasha
>>>>>
>>>>> On 7/18/2011 5:33 PM, Brad Wetmore wrote:
>>>>>> (Apologies to folks without access to the older sources.)
>>>>>>
>>>>>>
>>>>>> On 07/18/11 15:00, Sean Mullan wrote:
>>>>>>> On 7/18/11 5:35 PM, Alexandre Boulgakov wrote:
>>>>>>>> Is there an easy way to see when a class was added to the JDK?
>>>>>>> For standard API classes, you can use the @since javadoc tag which
>>>>>>> will indicate
>>>>>>> the release it was first introduced in.
>>>>>> Standard, exported API classes.  Some of the underlying support
>>>>>> classes for API packages like java.*.* weren't always @since'd properly.
>>>>>>
>>>>>>> For internal classes, there is no easy way, since most don't have an
>>>>>>> @since tag.
>>>>>>> I would probably write a script that checks the rt.jar of each of
>>>>>>> the JREs that
>>>>>>> are archived at /java/re/jdk. The pathnames should be fairly
>>>>>>> consistent, just
>>>>>>> the version number is different.
>>>>>> Don't know which classes you're talking about here, but some classes
>>>>>> started out in other jar files and eventually wound up in rt.jar.
>>>>>> Also, some files live in files other than rt.jar.  I usually go to
>>>>>> the source when looking for something.  If it's originally from
>>>>>> JSSE/JGSS/JCE, you'll need to look on our restricted access machine.
>>>>>>
>>>>>> When I'm looking for something that is in the jdk/j2se workspaces, I
>>>>>> go right to the old Codemgr data, specifically the nametable file,
>>>>>> because many times the files you want may be in a src/<arch>/classes
>>>>>> instead of src/share/classes.
>>>>>>
>>>>>> % grep -i SunMSCAPI.java
>>>>>> <RE-repository>/5.0/latest/ws/j2se/Codemgr_wsdata/nametable
>>>>>>
>>>>>> % grep -i SunMSCAPI.java
>>>>>> <RE-repository>/6.0/latest/ws/j2se/Codemgr_wsdata/nametable
>>>>>> src/windows/classes/sun/security/mscapi/SunMSCAPI.java ada8dbe4
>>>>>> a217f6b0 6c833bd3 d4ef32be
>>>>>>
>>>>>> That will usually give you a good starting point.
>>>>>>
>>>>>> Brad
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Depending on rt.jar or not.
>>>>>>
>>>>>>> Chris, do you have any other ideas?
>>>>>>>
>>>>>>> --Sean



More information about the security-dev mailing list