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

Wed Aug 3 23:49:53 UTC 2011

Oops, I missed this.

I don't think we can modify serialVersionUID value for backward
compatibility.

Thanks,
Xuelei

On 8/4/2011 7:39 AM, Alexandre Boulgakov wrote:
> Ping..?
> 
> -Sasha
> 
> On 7/27/2011 11:22 AM, Alexandre Boulgakov wrote:
>> Should I just use the newest serialVersionUID for both of them?
>>
>> -Sasha
>>
>> On 7/26/2011 10:31 AM, Alexandre Boulgakov wrote:
>>> 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