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

2011-08-18 Thread 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

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

2011-08-17 Thread 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/comm

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

2011-08-16 Thread 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 a

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

2011-08-10 Thread 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

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

2011-08-10 Thread 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 a

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

2011-08-04 Thread 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.P11TlsPrfG

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

2011-08-04 Thread 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 cl

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

2011-08-04 Thread 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.P11TlsPr

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

2011-08-04 Thread 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 _1

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

2011-08-03 Thread 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. $ c

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

2011-08-03 Thread 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, Xu

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

2011-08-03 Thread 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.P11SecretK

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

2011-08-03 Thread 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 bot

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

2011-08-03 Thread 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

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

2011-08-03 Thread 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 the

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

2011-07-27 Thread 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 tha

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

2011-07-26 Thread 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: -78282

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

2011-07-20 Thread 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

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

2011-07-20 Thread 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 = -

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

2011-07-20 Thread 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 i

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

2011-07-20 Thread 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 o

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

2011-07-19 Thread 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 war

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

2011-07-19 Thread 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

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

2011-07-19 Thread 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

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

2011-07-19 Thread 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,

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

2011-07-18 Thread 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.securi

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

2011-07-18 Thread 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 r

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

2011-07-18 Thread 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 hav

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

2011-07-18 Thread 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

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

2011-07-18 Thread 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)

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

2011-07-18 Thread 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 omi

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

2011-07-15 Thread 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 reason

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

2011-07-15 Thread 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 Seriali

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

2011-07-12 Thread 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 > >

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

2011-07-12 Thread 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/706

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

2011-07-12 Thread 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/bugd

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

2011-07-11 Thread 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

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

2011-07-11 Thread 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 warni

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

2011-07-11 Thread 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 preven