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

[Alexandre Boulgakov]

This is actually a complicated question. If a user sets 
JAVAC_MAX_WARNINGS=true globally, there are two things we can do:
We can listen to the user and exit with an error the moment one of these 
files comes along (in this case, we know there will be [deprecation] 
warnings), or we can ignore the user's request for the particular 
makefile by resetting JAVAC_MAX_WARNINGS=false.


I went with the latter simply because it allows for a full build with 
"make JAVAC_MAX_WARNINGS=true", which makes for easy bookkeeping.


-Sasha

On 8/17/2011 8:37 PM, Brad Wetmore wrote:

This is a little late, but catching up on a bunch of back email.

Why did you add:

+JAVAC_MAX_WARNINGS = false

in places like:

+JAVAC_MAX_WARNINGS=false
+JAVAC_LINT_OPTIONS=-Xlint:all,-deprecation
+JAVAC_WARNINGS_FATAL=true

The default is false (NOP) already:

jdk/make/common/shared/Defs-java.gmk

ifeq ($(JAVAC_MAX_WARNINGS), true)
  JAVAC_LINT_OPTIONS += -Xlint:all
endif

Just seems like additional overhead for the maintainer to understand, 
instead of just the two lines.


Brad




On 7/20/2011 10:28 AM, Alexandre Boulgakov wrote:

"JAVAC_MAX_WARNINGS = true" is the same as "JAVAC_LINT_OPTIONS =
-Xlint:all", so the only warnings being ignored are deprecation 
warnings.


-Sasha

On 7/19/2011 8:10 PM, Xuelei Fan wrote:

About the makefile. In some makefiles, you added:

JAVAC_MAX_WARNINGS = false
JAVAC_LINT_OPTIONS = -Xlint:all,-deprecation
JAVAC_WARNINGS_FATAL = true

But some others, the more strict rules are applied:
JAVAC_MAX_WARNINGS = true
JAVAC_WARNINGS_FATAL = true

What's the underlying warnings that we cannot always use the following
options:
JAVAC_MAX_WARNINGS = true
JAVAC_WARNINGS_FATAL = true


Thanks,
Xuelei (Andrew)

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.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) 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//classes
instead of src/share/classes.

% grep -i SunMSCAPI.java
/5.0/latest/ws/j2se/Codemgr_wsdata/nametable

% grep -i SunMSCAPI.java
/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

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

[Brad Wetmore]

This is a little late, but catching up on a bunch of back email.

Why did you add:

+JAVAC_MAX_WARNINGS = false

in places like:

+JAVAC_MAX_WARNINGS=false
+JAVAC_LINT_OPTIONS=-Xlint:all,-deprecation
+JAVAC_WARNINGS_FATAL=true

The default is false (NOP) already:

jdk/make/common/shared/Defs-java.gmk

ifeq ($(JAVAC_MAX_WARNINGS), true)
  JAVAC_LINT_OPTIONS += -Xlint:all
endif

Just seems like additional overhead for the maintainer to understand, 
instead of just the two lines.


Brad




On 7/20/2011 10:28 AM, Alexandre Boulgakov wrote:

"JAVAC_MAX_WARNINGS = true" is the same as "JAVAC_LINT_OPTIONS =
-Xlint:all", so the only warnings being ignored are deprecation warnings.

-Sasha

On 7/19/2011 8:10 PM, Xuelei Fan wrote:

About the makefile. In some makefiles, you added:

JAVAC_MAX_WARNINGS = false
JAVAC_LINT_OPTIONS = -Xlint:all,-deprecation
JAVAC_WARNINGS_FATAL = true

But some others, the more strict rules are applied:
JAVAC_MAX_WARNINGS = true
JAVAC_WARNINGS_FATAL = true

What's the underlying warnings that we cannot always use the following
options:
JAVAC_MAX_WARNINGS = true
JAVAC_WARNINGS_FATAL = true


Thanks,
Xuelei (Andrew)

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.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) 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//classes
instead of src/share/classes.

% grep -i SunMSCAPI.java
/5.0/latest/ws/j2se/Codemgr_wsdata/nametable

% grep -i SunMSCAPI.java
/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

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

[Weijun Wang]

My 2 cents:

@SuppressWarnings("serial") is never a good idea. There will still be a 
serialver automatically computed and you won't be able to control it.


Will the code changes go to jdk8 only? If so, I guess you should use the 
value calculated from jdk7 codes. If the values on jdk6 and jdk7 are 
already different, sorry, that means there is already a compatibility 
problem made and you won't be able to fix it. We can now only make sure 
jdk7 and jdk8 are compatible.


Ultimately you can write small test to see if serialized form from one 
version can be accepted by another version.


Thanks
Max


On 08/04/2011 08:10 AM, Alexandre Boulgakov wrote:

Those were the ones I was talking about- the serialVersionUIDs I
mentioned were the ones generated by serialver.exe. The webrev doesn't
have any of the pkcs11 changes yet (including the added serialVersionUIDs).

I'll wait for Brad's or Valerie's response.

Thanks,
Sasha

On 8/3/2011 5:07 PM, Xuelei Fan wrote:

On 8/4/2011 7:52 AM, Alexandre Boulgakov wrote:

There is currently no serialVersionUID defined for these classes. Do you
mean that we cannot add one for backwards compatibility?

My answers only apply to you latest e-mail about the serialVersionUID
update in sun.security.pkcs11.P11Key.P11SecretKey and
sun.security.pkcs11.P11TlsPrfGenerator$1. I think you need to use the
current values unless you get an approval from PKCS11 owner.

We may need to consider more for these classes without serialVersionUID,
I will look at your webrev again if I can get some time today. Normally,
I think if there is a new attribute in a class, it is OK to add a new
serialVersionUID value.

Xuelei


If so, would
the best solution be to add an @SuppressWarnings("serial") annotation to
these classes?

Thanks,
Sasha

On 8/3/2011 4:49 PM, Xuelei Fan wrote:

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@oracle.com wrote:

On Jul 21, 2011, at 1:25 AM, Alexandre
Boulgakov 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.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

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

[Xuelei Fan]

On 8/11/2011 7:07 AM, Alexandre Boulgakov wrote:
> Here's an updated webrev:
> http://cr.openjdk.java.net/~mduigou/7064075/3/webrev/
> 
> I've fixed build warnings in the pkcs11 files
> (make/sun/security/pkcs11/Makefile and
> src/share/classes/sun/security/pkcs11/*.java).
> 
> Please review these changes to pkcs11. (The changes in the other files
> have already been reviewed in this thread.)
> 
I only looked at PKCS11 changes this time. Looks fine to me.

Thanks,
Xuelei

> I would really appreciate a quick review since my internship is almost
> over - I am leaving next week - and I wanted to get this changeset in
> before I leave.
> 
> Thanks,
> 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.
> 
> Thanks,
> Sasha

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

[Alexandre Boulgakov]

Here's an updated webrev: http://cr.openjdk.java.net/~mduigou/7064075/3/webrev/ 

I've fixed build warnings in the pkcs11 files 
(make/sun/security/pkcs11/Makefile and 
src/share/classes/sun/security/pkcs11/*.java). 

Please review these changes to pkcs11. (The changes in the other files have 
already been reviewed in this thread.) 

I would really appreciate a quick review since my internship is almost over - I 
am leaving next week - and I wanted to get this changeset in before I leave. 

Thanks, 
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. 

Thanks, 
Sasha 

Re: final inner class not final (was Re: Code review request: 7064075 Security libraries don't build with javac -Xlint:all, -deprecation -Werror)

[Weijun Wang]

No, it isn't that complicated.

Take another class sun.security.jca.ProviderList$1 for example:

6-b54: 1151354171352296389L
6-b55: -8369942687198303343L (6219964)
6u3-latest: -8369942687198303343L
6u4-latest: 1151354171352296389L (6520152)

So the times are connect. sun.security.pkcs11.P11TlsPrfGenerator$1 is a 
JCE class which is inside the ext/sunpkcs11.jar. Most likely that jar 
was only compiled using the new javac from 6u11.


Thanks
Max



On 08/05/2011 10:07 AM, David Holmes wrote:

Interesting - 6520152 was fixed in 6u4, so it would appear that this got
turned on again at some stage and then off again in 6u11. Though I don't
see a bug fix in 6u11 that would cover this.

David

Tom Rodriguez said the following on 08/05/11 04:29:

I think that's a javac issue. The inner class has an attribute which
describes the access flags that the VM should report and that changed
in _11. Here it is in _10.

Attr(#25) { // InnerClasses
[] { // InnerClasses
#4 #0 #0 24;
}
} // end InnerClasses

and here's _11:

Attr(#25) { // InnerClasses
[] { // InnerClasses
#4 #0 #0 8;
}
} // end InnerClasses

tom

On Aug 3, 2011, at 7:24 PM, Weijun Wang wrote:


serialVersionUID warnings for classes that have had different
generated serialVersionUID's in the past.

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

I find out the reason:

Since 6u11, a final inner class is *not* final anymore.

$ cat A4.java
import java.lang.reflect.Modifier;
public class A4 {
public static void main(String[] args) throws Exception {
Class c = Class.forName(args[0]);
System.out.println(c.getModifiers() & Modifier.FINAL);
}
}
$ java A4 java.lang.String
16
$ java A4 'sun.security.pkcs11.P11TlsPrfGenerator$1'
0

getModifiers() is a native method, and calls JVM_GetClassModifiers.
Is this an intended change?

FYI, the class is defined as

private static final SecretKey NULL_KEY = new SecretKey() {


in
http://hg.openjdk.java.net/jdk8/tl/jdk/file/809e8db0c142/src/share/classes/sun/security/pkcs11/P11TlsPrfGenerator.java,
line 100.


Thanks
Max

Re: final inner class not final (was Re: Code review request: 7064075 Security libraries don't build with javac -Xlint:all, -deprecation -Werror)

[David Holmes]

Interesting -  6520152 was fixed in 6u4, so it would appear that this 
got turned on again at some stage and then off again in 6u11. Though I 
don't see a bug fix in 6u11 that would cover this.


David

Tom Rodriguez said the following on 08/05/11 04:29:

I think that's a javac issue.  The inner class has an attribute which describes 
the access flags that the VM should report and that changed in _11.  Here it is 
in _10.

Attr(#25) { // InnerClasses
  [] { // InnerClasses
#4 #0 #0 24;
  }
} // end InnerClasses

and here's _11:

Attr(#25) { // InnerClasses
  [] { // InnerClasses
#4 #0 #0 8;
  }
} // end InnerClasses

tom

On Aug 3, 2011, at 7:24 PM, Weijun Wang wrote:


serialVersionUID warnings for classes that have had different
generated serialVersionUID's in the past.

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

I find out the reason:

Since 6u11, a final inner class is *not* final anymore.

$ cat A4.java
import java.lang.reflect.Modifier;
public class A4 {
   public static void main(String[] args) throws Exception {
   Class c = Class.forName(args[0]);
   System.out.println(c.getModifiers() & Modifier.FINAL);
   }
}
$ java A4 java.lang.String
16
$ java A4 'sun.security.pkcs11.P11TlsPrfGenerator$1'
0

getModifiers() is a native method, and calls JVM_GetClassModifiers. Is this an 
intended change?

FYI, the class is defined as

  private static final SecretKey NULL_KEY = new SecretKey() {
   

in 
http://hg.openjdk.java.net/jdk8/tl/jdk/file/809e8db0c142/src/share/classes/sun/security/pkcs11/P11TlsPrfGenerator.java,
 line 100.


Thanks
Max

Re: final inner class not final (was Re: Code review request: 7064075 Security libraries don't build with javac -Xlint:all, -deprecation -Werror)

[David Holmes]

Max,

See 6520152. For backward compatability anonymous inner classes don't 
have the ACC_FINAL bit set.


David

Weijun Wang said the following on 08/04/11 12:24:

serialVersionUID warnings for classes that have had different
generated serialVersionUID's in the past.

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


I find out the reason:

Since 6u11, a final inner class is *not* final anymore.

$ cat A4.java
import java.lang.reflect.Modifier;
public class A4 {
public static void main(String[] args) throws Exception {
Class c = Class.forName(args[0]);
System.out.println(c.getModifiers() & Modifier.FINAL);
}
}
$ java A4 java.lang.String
16
$ java A4 'sun.security.pkcs11.P11TlsPrfGenerator$1'
0

getModifiers() is a native method, and calls JVM_GetClassModifiers. Is 
this an intended change?


FYI, the class is defined as

   private static final SecretKey NULL_KEY = new SecretKey() {


in 
http://hg.openjdk.java.net/jdk8/tl/jdk/file/809e8db0c142/src/share/classes/sun/security/pkcs11/P11TlsPrfGenerator.java, 
line 100.



Thanks
Max

Re: final inner class not final (was Re: Code review request: 7064075 Security libraries don't build with javac -Xlint:all, -deprecation -Werror)

[Tom Rodriguez]

I think that's a javac issue.  The inner class has an attribute which describes 
the access flags that the VM should report and that changed in _11.  Here it is 
in _10.

Attr(#25) { // InnerClasses
  [] { // InnerClasses
#4 #0 #0 24;
  }
} // end InnerClasses

and here's _11:

Attr(#25) { // InnerClasses
  [] { // InnerClasses
#4 #0 #0 8;
  }
} // end InnerClasses

tom

On Aug 3, 2011, at 7:24 PM, Weijun Wang wrote:

>> serialVersionUID warnings for classes that have had different
>> generated serialVersionUID's in the past.
 sun.security.pkcs11.P11TlsPrfGenerator$1
 -currently: -8090049519656411362L;
 -JDK 6: -3305145912345854189L;
> 
> I find out the reason:
> 
> Since 6u11, a final inner class is *not* final anymore.
> 
> $ cat A4.java
> import java.lang.reflect.Modifier;
> public class A4 {
>public static void main(String[] args) throws Exception {
>Class c = Class.forName(args[0]);
>System.out.println(c.getModifiers() & Modifier.FINAL);
>}
> }
> $ java A4 java.lang.String
> 16
> $ java A4 'sun.security.pkcs11.P11TlsPrfGenerator$1'
> 0
> 
> getModifiers() is a native method, and calls JVM_GetClassModifiers. Is this 
> an intended change?
> 
> FYI, the class is defined as
> 
>   private static final SecretKey NULL_KEY = new SecretKey() {
>
> 
> in 
> http://hg.openjdk.java.net/jdk8/tl/jdk/file/809e8db0c142/src/share/classes/sun/security/pkcs11/P11TlsPrfGenerator.java,
>  line 100.
> 
> 
> Thanks
> Max

final inner class not final (was Re: Code review request: 7064075 Security libraries don't build with javac -Xlint:all, -deprecation -Werror)

[Weijun Wang]

serialVersionUID warnings for classes that have had different
generated serialVersionUID's in the past.

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


I find out the reason:

Since 6u11, a final inner class is *not* final anymore.

$ cat A4.java
import java.lang.reflect.Modifier;
public class A4 {
public static void main(String[] args) throws Exception {
Class c = Class.forName(args[0]);
System.out.println(c.getModifiers() & Modifier.FINAL);
}
}
$ java A4 java.lang.String
16
$ java A4 'sun.security.pkcs11.P11TlsPrfGenerator$1'
0

getModifiers() is a native method, and calls JVM_GetClassModifiers. Is 
this an intended change?


FYI, the class is defined as

   private static final SecretKey NULL_KEY = new SecretKey() {


in 
http://hg.openjdk.java.net/jdk8/tl/jdk/file/809e8db0c142/src/share/classes/sun/security/pkcs11/P11TlsPrfGenerator.java, 
line 100.



Thanks
Max

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

[Alexandre Boulgakov]

Those were the ones I was talking about- the serialVersionUIDs I 
mentioned were the ones generated by serialver.exe. The webrev doesn't 
have any of the pkcs11 changes yet (including the added serialVersionUIDs).


I'll wait for Brad's or Valerie's response.

Thanks,
Sasha

On 8/3/2011 5:07 PM, Xuelei Fan wrote:

On 8/4/2011 7:52 AM, Alexandre Boulgakov wrote:

There is currently no serialVersionUID defined for these classes. Do you
mean that we cannot add one for backwards compatibility?

My answers only apply to you latest e-mail about the serialVersionUID
update in sun.security.pkcs11.P11Key.P11SecretKey and
sun.security.pkcs11.P11TlsPrfGenerator$1. I think you need to use the
current values unless you get an approval from PKCS11 owner.

We may need to consider more for these classes without serialVersionUID,
I will look at your webrev again if I can get some time today. Normally,
I think if there is a new attribute in a class, it is OK to add a new
serialVersionUID value.

Xuelei


If so, would
the best solution be to add an @SuppressWarnings("serial") annotation to
these classes?

Thanks,
Sasha

On 8/3/2011 4:49 PM, Xuelei Fan wrote:

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@oracle.com wrote:

On Jul 21, 2011, at 1:25 AM, Alexandre
Boulgakovwrote:


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.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) 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 s

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

[Xuelei Fan]

On 8/4/2011 7:52 AM, Alexandre Boulgakov wrote:
> There is currently no serialVersionUID defined for these classes. Do you
> mean that we cannot add one for backwards compatibility?
My answers only apply to you latest e-mail about the serialVersionUID
update in sun.security.pkcs11.P11Key.P11SecretKey and
sun.security.pkcs11.P11TlsPrfGenerator$1. I think you need to use the
current values unless you get an approval from PKCS11 owner.

We may need to consider more for these classes without serialVersionUID,
I will look at your webrev again if I can get some time today. Normally,
I think if there is a new attribute in a class, it is OK to add a new
serialVersionUID value.

Xuelei

> If so, would
> the best solution be to add an @SuppressWarnings("serial") annotation to
> these classes?
> 
> Thanks,
> Sasha
> 
> On 8/3/2011 4:49 PM, Xuelei Fan wrote:
>> 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@oracle.com wrote:
>> On Jul 21, 2011, at 1:25 AM, Alexandre
>> Boulgakov   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.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.

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

[Xuelei Fan]

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@oracle.com wrote:

 On Jul 21, 2011, at 1:25 AM, Alexandre
 Boulgakov  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.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) 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 suppor

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

[Alexandre Boulgakov]

There is currently no serialVersionUID defined for these classes. Do you 
mean that we cannot add one for backwards compatibility? If so, would 
the best solution be to add an @SuppressWarnings("serial") annotation to 
these classes?


Thanks,
Sasha

On 8/3/2011 4:49 PM, Xuelei Fan wrote:

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@oracle.com wrote:

On Jul 21, 2011, at 1:25 AM, Alexandre
Boulgakov   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.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) 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 th

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

[Alexandre Boulgakov]

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@oracle.com wrote:


On Jul 21, 2011, at 1:25 AM, Alexandre 
Boulgakov  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.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) 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//classes

instead of src/share/classes.

% grep -i SunMSCAPI.java
/5

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

[Alexandre Boulgakov]

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@oracle.com wrote:


On Jul 21, 2011, at 1:25 AM, Alexandre 
Boulgakov  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.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) 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//classes

instead of src/share/classes.

% grep -i SunMSCAPI.java
/5.0/latest/ws/j2se/Codemgr_wsdata/nametable

% grep -i SunMSCAPI.java
/6

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

[Alexandre Boulgakov]

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@oracle.com wrote:


On Jul 21, 2011, at 1:25 AM, Alexandre 
Boulgakov  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.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) 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//classes
instead of src/share/classes.

% grep -i SunMSCAPI.java
/5.0/latest/ws/j2se/Codemgr_wsdata/nametable

% grep -i SunMSCAPI.java
/6.0/latest/ws/j2se/Codemgr_wsdata/nametable
src/windows/classes/sun/security/mscapi/SunMSCAPI.java ada8dbe4
a217f6b0 6c833bd3 d4ef32be

That will u

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

[xuelei....@oracle.com]

On Jul 21, 2011, at 1:25 AM, Alexandre Boulgakov 
 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.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) 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//classes
> instead of src/share/classes.
> 
> % grep -i SunMSCAPI.java
> /5.0/latest/ws/j2se/Codemgr_wsdata/nametable
> 
> % grep -i SunMSCAPI.java
> /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

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

[Alexandre Boulgakov]

"JAVAC_MAX_WARNINGS = true" is the same as "JAVAC_LINT_OPTIONS = 
-Xlint:all", so the only warnings being ignored are deprecation warnings.


-Sasha

On 7/19/2011 8:10 PM, Xuelei Fan wrote:

About the makefile. In some makefiles, you added:

JAVAC_MAX_WARNINGS = false
JAVAC_LINT_OPTIONS = -Xlint:all,-deprecation
JAVAC_WARNINGS_FATAL = true

But some others, the more strict rules are applied:
JAVAC_MAX_WARNINGS = true
JAVAC_WARNINGS_FATAL = true

What's the underlying warnings that we cannot always use the following
options:
JAVAC_MAX_WARNINGS = true
JAVAC_WARNINGS_FATAL = true


Thanks,
Xuelei (Andrew)

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.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) 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//classes
instead of src/share/classes.

% grep -i SunMSCAPI.java
/5.0/latest/ws/j2se/Codemgr_wsdata/nametable

% grep -i SunMSCAPI.java
/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

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

[Alexandre Boulgakov]

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.


-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.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) 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//classes
instead of src/share/classes.

% grep -i SunMSCAPI.java
/5.0/latest/ws/j2se/Codemgr_wsdata/nametable

% grep -i SunMSCAPI.java
/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

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

[Sean Mullan]

Hi Sasha,

The updates look fine to me.

--Sean

On 7/18/11 9: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.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) 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//classes 
>> instead of src/share/classes.
>>
>> % grep -i SunMSCAPI.java 
>> /5.0/latest/ws/j2se/Codemgr_wsdata/nametable
>>
>> % grep -i SunMSCAPI.java
>> /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

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

[Xuelei Fan]

About the makefile. In some makefiles, you added:

   JAVAC_MAX_WARNINGS = false
   JAVAC_LINT_OPTIONS = -Xlint:all,-deprecation
   JAVAC_WARNINGS_FATAL = true

But some others, the more strict rules are applied:
   JAVAC_MAX_WARNINGS = true
   JAVAC_WARNINGS_FATAL = true

What's the underlying warnings that we cannot always use the following
options:
   JAVAC_MAX_WARNINGS = true
   JAVAC_WARNINGS_FATAL = true


Thanks,
Xuelei (Andrew)

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.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) 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//classes
>>> instead of src/share/classes.
>>>
>>> % grep -i SunMSCAPI.java
>>> /5.0/latest/ws/j2se/Codemgr_wsdata/nametable
>>>
>>> % grep -i SunMSCAPI.java
>>> /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

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

[Xuelei Fan]

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.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) 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//classes
>>> instead of src/share/classes.
>>>
>>> % grep -i SunMSCAPI.java
>>> /5.0/latest/ws/j2se/Codemgr_wsdata/nametable
>>>
>>> % grep -i SunMSCAPI.java
>>> /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

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

[Alexandre Boulgakov]

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.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) 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//classes 
instead of src/share/classes.


% grep -i SunMSCAPI.java 
/5.0/latest/ws/j2se/Codemgr_wsdata/nametable


% grep -i SunMSCAPI.java
/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

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

[Chris Hegarty]

On 07/18/11 11:00 PM, 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.

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.

Chris, do you have any other ideas?


I did this process manually. Yes it's a pain, but it doesn't really take 
that long, 1-2 minutes per class. As Sean said, the pathnames are fairly 
consistent.


-Chris.


--Sean

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

[Alexandre Boulgakov]

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.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) 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//classes 
instead of src/share/classes.


% grep -i SunMSCAPI.java 
/5.0/latest/ws/j2se/Codemgr_wsdata/nametable


% grep -i SunMSCAPI.java
/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

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

[Brad Wetmore]

(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//classes instead of 
src/share/classes.


% grep -i SunMSCAPI.java 
/5.0/latest/ws/j2se/Codemgr_wsdata/nametable


% grep -i SunMSCAPI.java
/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

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

[Sean Mullan]

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.

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.

Chris, do you have any other ideas?

--Sean

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

[Alexandre Boulgakov]

Is there an easy way to see when a class was added to the JDK?

Thanks,
Sasha

On 7/18/2011 1:43 PM, Sean Mullan wrote:

On 7/18/11 1:17 PM, Alexandre Boulgakov wrote:

Please see my responses in-line.

Thanks for reviewing!

-Sasha

On 7/15/2011 6:18 AM, Chris Hegarty wrote:

On 07/15/11 01:19 PM, Sean Mullan wrote:

All the changes look good. I only have a couple of questions/comments:

1) How did you calculate the serialVersionUID for the classes that
had omitted
this field? Ideally you want to calculate it on a previous version of
the class,
for compatibility reasons. This is especially important for
Serializable objects
that are exposed via public APIs.

I added serialVersionUID to a bunch of public java lang/net/util
classes a wile back. I used jdk/bin/serialver to generate the
serialVersionUID and verified that it was the same for previous
versions of the class back to when the class was originally added. Not
such a problem when you have archived jdk's back to 1.1.8 in /java/re
in the US domain! Maybe Sasha could do something similar.

I used jdk/bin/serialver from JDK 7. Should I check if it's the same for
when the classes were originally added?

Yes, if it is the same as when the class was originally added to the JDK that
would most likely mean it has not changed since and it is safe to use.

--Sean


-Chris.


2) Consider adding a comment explaining the
"@SuppressWarnings("unchecked")"
annotations

That sounds good. I'll post another webrev once I've added the comments.

--Sean

On 7/12/11 4:16 PM, Alexandre Boulgakov wrote:

Since yesterday's webrev, I've changed some unchecked casts to use
Class.cast(Object) instead, per Dave's suggestion.
Updated webrev: http://cr.openjdk.java.net/~jjg/7064075.1/

Also, the original webrev was posted under the wrong bug ID, so it's
been moved to http://cr.openjdk.java.net/~jjg/7064075/.

Thanks,
Sasha

On 7/12/2011 1:03 PM, Brad Wetmore wrote:

Sean/Valerie/Max/Xuelei,


Hello Brad,

Could you please review these changes?

I'm swamped again, can one of you take a look at Sasha's changes?

Brad



On 7/11/2011 1:56 PM, Alexandre Boulgakov wrote:

Hello Brad,

Could you please review these changes?

Bug detail:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064075
webrev: http://cr.openjdk.java.net/~jjg/7076075/

Summary:

  * Small changes to Java files to remove most build warnings.
  * Small changes to relevant makefiles to prevent
reintroduction of
removed warnings.


Thanks,
Sasha Boulgakov

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

[Sean Mullan]

On 7/18/11 1:17 PM, Alexandre Boulgakov wrote:
> Please see my responses in-line.
> 
> Thanks for reviewing!
> 
> -Sasha
> 
> On 7/15/2011 6:18 AM, Chris Hegarty wrote:
>> On 07/15/11 01:19 PM, Sean Mullan wrote:
>>> All the changes look good. I only have a couple of questions/comments:
>>>
>>> 1) How did you calculate the serialVersionUID for the classes that 
>>> had omitted
>>> this field? Ideally you want to calculate it on a previous version of 
>>> the class,
>>> for compatibility reasons. This is especially important for 
>>> Serializable objects
>>> that are exposed via public APIs.
>>
>> I added serialVersionUID to a bunch of public java lang/net/util 
>> classes a wile back. I used jdk/bin/serialver to generate the 
>> serialVersionUID and verified that it was the same for previous 
>> versions of the class back to when the class was originally added. Not 
>> such a problem when you have archived jdk's back to 1.1.8 in /java/re 
>> in the US domain! Maybe Sasha could do something similar.
> I used jdk/bin/serialver from JDK 7. Should I check if it's the same for 
> when the classes were originally added?

Yes, if it is the same as when the class was originally added to the JDK that
would most likely mean it has not changed since and it is safe to use.

--Sean

>>
>> -Chris.
>>
>>>
>>> 2) Consider adding a comment explaining the 
>>> "@SuppressWarnings("unchecked")"
>>> annotations
> That sounds good. I'll post another webrev once I've added the comments.
>>>
>>> --Sean
>>>
>>> On 7/12/11 4:16 PM, Alexandre Boulgakov wrote:
 Since yesterday's webrev, I've changed some unchecked casts to use
 Class.cast(Object) instead, per Dave's suggestion.
 Updated webrev: http://cr.openjdk.java.net/~jjg/7064075.1/

 Also, the original webrev was posted under the wrong bug ID, so it's
 been moved to http://cr.openjdk.java.net/~jjg/7064075/.

 Thanks,
 Sasha

 On 7/12/2011 1:03 PM, Brad Wetmore wrote:
> Sean/Valerie/Max/Xuelei,
>
>> Hello Brad,
>>
>> Could you please review these changes?
>
> I'm swamped again, can one of you take a look at Sasha's changes?
>
> Brad
>
>
>
> On 7/11/2011 1:56 PM, Alexandre Boulgakov wrote:
>> Hello Brad,
>>
>> Could you please review these changes?
>>
>> Bug detail: 
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064075
>> webrev: http://cr.openjdk.java.net/~jjg/7076075/
>>
>> Summary:
>>
>>  * Small changes to Java files to remove most build warnings.
>>  * Small changes to relevant makefiles to prevent 
>> reintroduction of
>>removed warnings.
>>
>>
>> Thanks,
>> Sasha Boulgakov
>>

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

[Alexandre Boulgakov]

Please see my responses in-line.

Thanks for reviewing!

-Sasha

On 7/15/2011 6:18 AM, Chris Hegarty wrote:

On 07/15/11 01:19 PM, Sean Mullan wrote:

All the changes look good. I only have a couple of questions/comments:

1) How did you calculate the serialVersionUID for the classes that 
had omitted
this field? Ideally you want to calculate it on a previous version of 
the class,
for compatibility reasons. This is especially important for 
Serializable objects

that are exposed via public APIs.


I added serialVersionUID to a bunch of public java lang/net/util 
classes a wile back. I used jdk/bin/serialver to generate the 
serialVersionUID and verified that it was the same for previous 
versions of the class back to when the class was originally added. Not 
such a problem when you have archived jdk's back to 1.1.8 in /java/re 
in the US domain! Maybe Sasha could do something similar.
I used jdk/bin/serialver from JDK 7. Should I check if it's the same for 
when the classes were originally added?


-Chris.



2) Consider adding a comment explaining the 
"@SuppressWarnings("unchecked")"

annotations

That sounds good. I'll post another webrev once I've added the comments.


--Sean

On 7/12/11 4:16 PM, Alexandre Boulgakov wrote:

Since yesterday's webrev, I've changed some unchecked casts to use
Class.cast(Object) instead, per Dave's suggestion.
Updated webrev: http://cr.openjdk.java.net/~jjg/7064075.1/

Also, the original webrev was posted under the wrong bug ID, so it's
been moved to http://cr.openjdk.java.net/~jjg/7064075/.

Thanks,
Sasha

On 7/12/2011 1:03 PM, Brad Wetmore wrote:

Sean/Valerie/Max/Xuelei,


Hello Brad,

Could you please review these changes?


I'm swamped again, can one of you take a look at Sasha's changes?

Brad



On 7/11/2011 1:56 PM, Alexandre Boulgakov wrote:

Hello Brad,

Could you please review these changes?

Bug detail: 
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064075

webrev: http://cr.openjdk.java.net/~jjg/7076075/

Summary:

 * Small changes to Java files to remove most build warnings.
 * Small changes to relevant makefiles to prevent 
reintroduction of

   removed warnings.


Thanks,
Sasha Boulgakov

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

[Chris Hegarty]

On 07/15/11 01:19 PM, Sean Mullan wrote:

All the changes look good. I only have a couple of questions/comments:

1) How did you calculate the serialVersionUID for the classes that had omitted
this field? Ideally you want to calculate it on a previous version of the class,
for compatibility reasons. This is especially important for Serializable objects
that are exposed via public APIs.


I added serialVersionUID to a bunch of public java lang/net/util classes 
a wile back. I used jdk/bin/serialver to generate the serialVersionUID 
and verified that it was the same for previous versions of the class 
back to when the class was originally added. Not such a problem when you 
have archived jdk's back to 1.1.8 in /java/re in the US domain! Maybe 
Sasha could do something similar.


-Chris.



2) Consider adding a comment explaining the "@SuppressWarnings("unchecked")"
annotations

--Sean

On 7/12/11 4:16 PM, Alexandre Boulgakov wrote:

Since yesterday's webrev, I've changed some unchecked casts to use
Class.cast(Object) instead, per Dave's suggestion.
Updated webrev: http://cr.openjdk.java.net/~jjg/7064075.1/

Also, the original webrev was posted under the wrong bug ID, so it's
been moved to http://cr.openjdk.java.net/~jjg/7064075/.

Thanks,
Sasha

On 7/12/2011 1:03 PM, Brad Wetmore wrote:

Sean/Valerie/Max/Xuelei,


Hello Brad,

Could you please review these changes?


I'm swamped again, can one of you take a look at Sasha's changes?

Brad



On 7/11/2011 1:56 PM, Alexandre Boulgakov wrote:

Hello Brad,

Could you please review these changes?

Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064075
webrev: http://cr.openjdk.java.net/~jjg/7076075/

Summary:

 * Small changes to Java files to remove most build warnings.
 * Small changes to relevant makefiles to prevent reintroduction of
   removed warnings.


Thanks,
Sasha Boulgakov

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

[Sean Mullan]

All the changes look good. I only have a couple of questions/comments:

1) How did you calculate the serialVersionUID for the classes that had omitted
this field? Ideally you want to calculate it on a previous version of the class,
for compatibility reasons. This is especially important for Serializable objects
that are exposed via public APIs.

2) Consider adding a comment explaining the "@SuppressWarnings("unchecked")"
annotations

--Sean

On 7/12/11 4:16 PM, Alexandre Boulgakov wrote:
> Since yesterday's webrev, I've changed some unchecked casts to use 
> Class.cast(Object) instead, per Dave's suggestion.
> Updated webrev: http://cr.openjdk.java.net/~jjg/7064075.1/
> 
> Also, the original webrev was posted under the wrong bug ID, so it's 
> been moved to http://cr.openjdk.java.net/~jjg/7064075/.
> 
> Thanks,
> Sasha
> 
> On 7/12/2011 1:03 PM, Brad Wetmore wrote:
>> Sean/Valerie/Max/Xuelei,
>>
>>> Hello Brad,
>>>
>>> Could you please review these changes?
>>
>> I'm swamped again, can one of you take a look at Sasha's changes?
>>
>> Brad
>>
>>
>>
>> On 7/11/2011 1:56 PM, Alexandre Boulgakov wrote:
>>> Hello Brad,
>>>
>>> Could you please review these changes?
>>>
>>> Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064075
>>> webrev: http://cr.openjdk.java.net/~jjg/7076075/
>>>
>>> Summary:
>>>
>>> * Small changes to Java files to remove most build warnings.
>>> * Small changes to relevant makefiles to prevent reintroduction of
>>>   removed warnings.
>>>
>>>
>>> Thanks,
>>> Sasha Boulgakov
>>>

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

[Sean Mullan]

I'll look at it tomorrow, unless Valerie/Max/Xuelei beat me to it.

--Sean

On 7/12/11 4:03 PM, Brad Wetmore wrote:
> Sean/Valerie/Max/Xuelei,
> 
>  > Hello Brad,
>  >
>  > Could you please review these changes?
> 
> I'm swamped again, can one of you take a look at Sasha's changes?
> 
> Brad
> 
> 
> 
> On 7/11/2011 1:56 PM, Alexandre Boulgakov wrote:
>> Hello Brad,
>>
>> Could you please review these changes?
>>
>> Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064075
>> webrev: http://cr.openjdk.java.net/~jjg/7076075/
>>
>> Summary:
>>
>> * Small changes to Java files to remove most build warnings.
>> * Small changes to relevant makefiles to prevent reintroduction of
>>   removed warnings.
>>
>>
>> Thanks,
>> Sasha Boulgakov
>>

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

[Alexandre Boulgakov]

Since yesterday's webrev, I've changed some unchecked casts to use 
Class.cast(Object) instead, per Dave's suggestion.

Updated webrev: http://cr.openjdk.java.net/~jjg/7064075.1/

Also, the original webrev was posted under the wrong bug ID, so it's 
been moved to http://cr.openjdk.java.net/~jjg/7064075/.


Thanks,
Sasha

On 7/12/2011 1:03 PM, Brad Wetmore wrote:

Sean/Valerie/Max/Xuelei,

> Hello Brad,
>
> Could you please review these changes?

I'm swamped again, can one of you take a look at Sasha's changes?

Brad



On 7/11/2011 1:56 PM, Alexandre Boulgakov wrote:

Hello Brad,

Could you please review these changes?

Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064075
webrev: http://cr.openjdk.java.net/~jjg/7076075/

Summary:

* Small changes to Java files to remove most build warnings.
* Small changes to relevant makefiles to prevent reintroduction of
  removed warnings.


Thanks,
Sasha Boulgakov

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

[Brad Wetmore]

Sean/Valerie/Max/Xuelei,

> Hello Brad,
>
> Could you please review these changes?

I'm swamped again, can one of you take a look at Sasha's changes?

Brad



On 7/11/2011 1:56 PM, Alexandre Boulgakov wrote:

Hello Brad,

Could you please review these changes?

Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064075
webrev: http://cr.openjdk.java.net/~jjg/7076075/

Summary:

* Small changes to Java files to remove most build warnings.
* Small changes to relevant makefiles to prevent reintroduction of
  removed warnings.


Thanks,
Sasha Boulgakov

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

[Alexandre Boulgakov]

Thanks, Dave. I didn't know that existed.

-Sasha

On 7/11/2011 3:39 PM, David Schlosnagle wrote:

On Mon, Jul 11, 2011 at 4:56 PM, Alexandre Boulgakov
  wrote:

Could you please review these changes?

Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064075
webrev: http://cr.openjdk.java.net/~jjg/7076075/

Summary:

Small changes to Java files to remove most build warnings.
Small changes to relevant makefiles to prevent reintroduction of removed
warnings.

Sasha,

You can avoid the need for the @SuppressWarnings("unchecked") in many
cases by using Class.cast(Object) instead of (T). For example, in
BlockCipherParamsCore.java line 97:
   93  T
getParameterSpec(Class  paramSpec)
   94 throws InvalidParameterSpecException
   95 {
   96 if (IvParameterSpec.class.isAssignableFrom(paramSpec)) {
   97 return paramSpec.cast(new IvParameterSpec(this.iv));
   98 } else {
   99 throw new InvalidParameterSpecException
  100 ("Inappropriate parameter specification");
  101 }
  102 }

Also see these locations as well:
BlockCipherParamsCore.java line 97
DHKeyFactory.java lines 153, 158, 171, 176
DHParameters.java line 103
OAEPParameters.java line 187
PBEParameters.java line 106
RC2Parameters.java line 185
GSSUtil.java line 352
SubjectComber.java line 60
DSAKeyFactory.java lines 195, 201, 220, 226
DSAParameters.java line 106
RSAKeyFactory.java lines 355, 360, 368, 372, 388

- Dave

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

[David Schlosnagle]

On Mon, Jul 11, 2011 at 4:56 PM, Alexandre Boulgakov
 wrote:
> Could you please review these changes?
>
> Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064075
> webrev: http://cr.openjdk.java.net/~jjg/7076075/
>
> Summary:
>
> Small changes to Java files to remove most build warnings.
> Small changes to relevant makefiles to prevent reintroduction of removed
> warnings.

Sasha,

You can avoid the need for the @SuppressWarnings("unchecked") in many
cases by using Class.cast(Object) instead of (T). For example, in
BlockCipherParamsCore.java line 97:
  93  T
getParameterSpec(Class paramSpec)
  94 throws InvalidParameterSpecException
  95 {
  96 if (IvParameterSpec.class.isAssignableFrom(paramSpec)) {
  97 return paramSpec.cast(new IvParameterSpec(this.iv));
  98 } else {
  99 throw new InvalidParameterSpecException
 100 ("Inappropriate parameter specification");
 101 }
 102 }

Also see these locations as well:
BlockCipherParamsCore.java line 97
DHKeyFactory.java lines 153, 158, 171, 176
DHParameters.java line 103
OAEPParameters.java line 187
PBEParameters.java line 106
RC2Parameters.java line 185
GSSUtil.java line 352
SubjectComber.java line 60
DSAKeyFactory.java lines 195, 201, 220, 226
DSAParameters.java line 106
RSAKeyFactory.java lines 355, 360, 368, 372, 388

- Dave

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

[Alexandre Boulgakov]

Hello Brad,

Could you please review these changes?

Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064075
webrev: http://cr.openjdk.java.net/~jjg/7076075/

Summary:

   * Small changes to Java files to remove most build warnings.
   * Small changes to relevant makefiles to prevent reintroduction of
 removed warnings.


Thanks,
Sasha Boulgakov