Re: Code review request: 8026235: keytool NSS test should use 64 bit lib on Solaris
Your changes look fine. Thanks. On 10/10/2013 01:56, Weijun Wang wrote: Hi Vinnie Please take a review at http://cr.openjdk.java.net/~weijun/8026235/webrev.00/ Thanks Max
Re: [8] Review Request for 8007292 : Add JavaFX internal packages to package.access
On 10/9/2013 6:18 PM, David Holmes wrote: cc'ing Joe Darcy. :) Joe: there is a try-with-resources question for you below ... On 9/10/2013 11:20 PM, Sean Mullan wrote: On 10/09/2013 05:14 AM, Erik Joelsson wrote: On 2013-10-09 06:33, David Holmes wrote: In the tool this code doesn't show correct use of try-with-resources: 51 try (BufferedReader br = new BufferedReader(new FileReader(args[0])); 52 BufferedWriter bw = new BufferedWriter(new FileWriter(args[1]))) { The FileReader and FileWriter should also be covered by TWR: try (FileReader fr = new FileReader(args[0]); BufferedReader br = new BufferedReader(fr); FileWriter fw = new FileWriter(args[1]); BufferedWriter bw = new BufferedWriter(fw)) { I'm not familiar with the try-with-resources, but calling close on a BufferedReader/writer will close the underlying reader/writer so nothing will be left open, will it not? That's what I thought as well. David? It maybe that I am overly pedantic with this but the issue is that with the original code if the BufferedReader/Writer constructors throw an exception then the FileReader/Writer that was already created would not be closed. The revised code accounts for this. Joe: what is best-practice here? I see a lot of examples of t-w-r where there is a set of chained I/O streams and only the outermost one is a t-w-r resource. And that seems wrong to me. It is a hazard (I thought I had published a blog entry on this very tropic, but apparently not). The most robust pattern is try(OriginalResource r1 = new OriginalResource; WrappingResource r2 = new WrappingResource(r1); AnnotherWrappingResource r3 = new WrappingResource(r2)) { ...} One thing to watch out for in this pattern is a non-idempotent close. Calling close on r3 will presumably propagate a close call to r2, and then r2 to r1. So give the desguaring the try-with-resource and the expected behavior of the wrapping in a normal termination situation, close on r3 gets called once, close on r2 gets called twice (first from the close on r3, second close from try-with-resources), and close on r1 gets called three times. HTH, -Joe
Re: [8] Request for Review: 8026052: cannot find symbol Proc
This looks harmless. But it looks more like a jtreg issue if it cannot deal with that slash. I haven't noticed it failing on Windows before. Can you point me to a jtr file. Thanks Max On 10/10/13 8:56 AM, Jason Uh wrote: Please review this fix. This changeset removes trailing slashes at the end of the test library paths in the jtreg @library tag, which can cause the tests to fail on Windows. webrev: http://cr.openjdk.java.net/~juh/8026052/webrev.00/ Thanks, Jason
hg: jdk8/tl/jdk: 8024354: Explicitly permit DoubleStream.sum()/average() implementations to use higher precision summation
Changeset: c13309f658e1 Author:darcy Date: 2013-10-09 18:31 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c13309f658e1 8024354: Explicitly permit DoubleStream.sum()/average() implementations to use higher precision summation Reviewed-by: mduigou, briangoetz ! src/share/classes/java/util/DoubleSummaryStatistics.java ! src/share/classes/java/util/stream/DoubleStream.java
hg: jdk8/tl/jdk: 8024709: TreeMap.DescendingKeyIterator 'remove' confuses iterator position
Changeset: eab3c09745b6 Author:bchristi Date: 2013-10-09 12:13 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/eab3c09745b6 8024709: TreeMap.DescendingKeyIterator 'remove' confuses iterator position Summary: Override remove() method in DescendingKeyIterator Reviewed-by: alanb, mduigou, psandoz ! src/share/classes/java/util/TreeMap.java ! test/java/util/Collection/MOAT.java
Re: [8] Review Request for 8007292 : Add JavaFX internal packages to package.access
cc'ing Joe Darcy. :) Joe: there is a try-with-resources question for you below ... On 9/10/2013 11:20 PM, Sean Mullan wrote: On 10/09/2013 05:14 AM, Erik Joelsson wrote: On 2013-10-09 06:33, David Holmes wrote: In the tool this code doesn't show correct use of try-with-resources: 51 try (BufferedReader br = new BufferedReader(new FileReader(args[0])); 52 BufferedWriter bw = new BufferedWriter(new FileWriter(args[1]))) { The FileReader and FileWriter should also be covered by TWR: try (FileReader fr = new FileReader(args[0]); BufferedReader br = new BufferedReader(fr); FileWriter fw = new FileWriter(args[1]); BufferedWriter bw = new BufferedWriter(fw)) { I'm not familiar with the try-with-resources, but calling close on a BufferedReader/writer will close the underlying reader/writer so nothing will be left open, will it not? That's what I thought as well. David? It maybe that I am overly pedantic with this but the issue is that with the original code if the BufferedReader/Writer constructors throw an exception then the FileReader/Writer that was already created would not be closed. The revised code accounts for this. Joe: what is best-practice here? I see a lot of examples of t-w-r where there is a set of chained I/O streams and only the outermost one is a t-w-r resource. And that seems wrong to me. Thanks, David Finally do we still use make/tools/Makefile in the new build? No, we don't. If you want to add old build support for this, you would also need to add usage of the tool to the correct old makefile. I don't think it's necessary to add this to the old build at this point. I'll post another webrev later in the day with these updates. Thanks, Sean
[8] Request for Review: 8026052: cannot find symbol Proc
Please review this fix. This changeset removes trailing slashes at the end of the test library paths in the jtreg @library tag, which can cause the tests to fail on Windows. webrev: http://cr.openjdk.java.net/~juh/8026052/webrev.00/ Thanks, Jason
[8] Request for Review: 8026233: test/sun/security/tools/keytool/StorePasswords.java needs to clean up files
Hi Vinnie, Could you please review this fix? The test sun/security/tools/keytool/StorePasswords.java can terminate with an error on Windows because of files not getting cleaned up, so this fix deletes the keystore file at the end of the test. webrev: http://cr.openjdk.java.net/~juh/8026233/webrev.00/ Thanks, Jason
Code review request: 8026235: keytool NSS test should use 64 bit lib on Solaris
Hi Vinnie Please take a review at http://cr.openjdk.java.net/~weijun/8026235/webrev.00/ Thanks Max
hg: jdk8/tl/jdk: 7189139: BigInteger's staticRandom field can be a source of bottlenecks.
Changeset: 673f8045311e Author:bpb Date: 2013-10-09 17:22 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/673f8045311e 7189139: BigInteger's staticRandom field can be a source of bottlenecks. Summary: Use ThreadLocalRandom instead of SecureRandom. Reviewed-by: shade, psandoz Contributed-by: Brian Burkhalter ! src/share/classes/java/math/BigInteger.java
hg: jdk8/tl/jdk: 5 new changesets
Changeset: 1597066b58ee Author:valeriep Date: 2013-10-08 11:07 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1597066b58ee 7196382: PKCS11 provider should support 2048-bit DH Summary: Query and enforce range checking using the values from native PKCS11 library. Reviewed-by: xuelei ! src/share/classes/com/sun/crypto/provider/DHParameterGenerator.java ! src/share/classes/sun/security/pkcs11/P11KeyPairGenerator.java + test/sun/security/pkcs11/KeyPairGenerator/TestDH2048.java Changeset: 3da8be8d13bf Author:valeriep Date: 2013-10-08 11:17 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3da8be8d13bf 8012900: CICO ignores AAD in GCM mode Summary: Change GCM decryption to not return result until tag verification passed Reviewed-by: xuelei ! src/share/classes/com/sun/crypto/provider/CipherBlockChaining.java ! src/share/classes/com/sun/crypto/provider/CipherCore.java ! src/share/classes/com/sun/crypto/provider/CipherFeedback.java ! src/share/classes/com/sun/crypto/provider/CounterMode.java ! src/share/classes/com/sun/crypto/provider/ElectronicCodeBook.java ! src/share/classes/com/sun/crypto/provider/FeedbackCipher.java ! src/share/classes/com/sun/crypto/provider/GCTR.java ! src/share/classes/com/sun/crypto/provider/GaloisCounterMode.java ! src/share/classes/com/sun/crypto/provider/OutputFeedback.java ! src/share/classes/com/sun/crypto/provider/PCBC.java ! src/share/classes/javax/crypto/CipherSpi.java + test/com/sun/crypto/provider/Cipher/AES/TestCICOWithGCMAndAAD.java Changeset: f4305254f92f Author:valeriep Date: 2013-10-08 11:35 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f4305254f92f 8014374: Cannot initialize "AES/GCM/NoPadding" on wrap/unseal on solaris with OracleUcrypto Summary: Removed OracleUcrypto provider regression tests from OpenJDK Reviewed-by: xuelei - test/com/oracle/security/ucrypto/TestAES.java - test/com/oracle/security/ucrypto/TestDigest.java - test/com/oracle/security/ucrypto/TestRSA.java - test/com/oracle/security/ucrypto/UcryptoTest.java Changeset: e044b0151858 Author:valeriep Date: 2013-10-08 14:41 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e044b0151858 8025967: addition of -Werror broke the old build Summary: Fixed and suppressed compiler warnings on rawtypes Reviewed-by: vinnie ! src/share/classes/com/sun/jndi/ldap/LdapCtxFactory.java ! src/share/classes/com/sun/jndi/ldap/LdapPoolManager.java ! src/share/classes/com/sun/net/ssl/internal/www/protocol/https/HttpsURLConnectionOldImpl.java ! src/share/classes/java/lang/instrument/Instrumentation.java ! src/share/classes/java/net/ContentHandler.java ! src/share/classes/javax/crypto/JceSecurityManager.java ! src/share/classes/sun/instrument/InstrumentationImpl.java ! src/share/classes/sun/net/www/content/image/gif.java ! src/share/classes/sun/net/www/content/image/jpeg.java ! src/share/classes/sun/net/www/content/image/png.java ! src/share/classes/sun/net/www/content/image/x_xbitmap.java ! src/share/classes/sun/net/www/content/image/x_xpixmap.java ! src/share/classes/sun/net/www/protocol/https/HttpsURLConnectionImpl.java ! src/share/classes/sun/reflect/misc/MethodUtil.java ! src/share/classes/sun/security/provider/AuthPolicyFile.java ! src/share/classes/sun/security/provider/SubjectCodeSource.java ! src/share/classes/sun/security/tools/jarsigner/Main.java ! src/share/classes/sun/security/tools/keytool/Main.java ! src/share/classes/sun/security/tools/policytool/PolicyTool.java Changeset: 7a7b73a40bb1 Author:valeriep Date: 2013-10-09 13:07 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7a7b73a40bb1 Merge - src/share/classes/com/sun/jdi/connect/package.html - src/share/classes/com/sun/jdi/connect/spi/package.html - src/share/classes/com/sun/jdi/event/package.html - src/share/classes/com/sun/jdi/package.html - src/share/classes/com/sun/jdi/request/package.html - src/share/classes/com/sun/management/package.html - src/share/classes/com/sun/tools/attach/package.html - src/share/classes/com/sun/tools/attach/spi/package.html - src/share/classes/com/sun/tools/jconsole/package.html
hg: jdk8/tl/jdk: 8016252: More defensive HashSet.readObject
Changeset: b86e6700266e Author:bpb Date: 2013-10-09 11:47 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b86e6700266e 8016252: More defensive HashSet.readObject Summary: Add data validation checks in readObject(). Reviewed-by: alanb, mduigou, chegar Contributed-by: Brian Burkhalter ! src/share/classes/java/util/HashSet.java + test/java/util/HashSet/Serialization.java
hg: jdk8/tl/jdk: 8024076: Incorrect 2 -> 4 year parsing and resolution in DateTimeFormatter
Changeset: d42fe440bda8 Author:rriggs Date: 2013-10-09 13:34 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d42fe440bda8 8024076: Incorrect 2 -> 4 year parsing and resolution in DateTimeFormatter Summary: Add appendValueReduced method based on a ChronoLocalDate to provide context for the value Reviewed-by: sherman Contributed-by: scolebou...@joda.org ! src/share/classes/java/time/format/DateTimeFormatterBuilder.java ! test/java/time/tck/java/time/format/TCKDateTimeFormatterBuilder.java ! test/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java ! test/java/time/test/java/time/format/TestReducedParser.java ! test/java/time/test/java/time/format/TestReducedPrinter.java
Re: [8] Review Request for 8007292 : Add JavaFX internal packages to package.access
Updated webrev: http://cr.openjdk.java.net/~mullan/webrevs/8007292/webrev.01/ Let me know if there are any more comments, otherwise I will plan to push tomorrow. Thanks, Sean On 10/09/2013 09:20 AM, Sean Mullan wrote: On 10/09/2013 05:14 AM, Erik Joelsson wrote: Hello Sean, On 2013-10-09 06:33, David Holmes wrote: Hi Sean, Not a full review. On 9/10/2013 5:52 AM, Sean Mullan wrote: Please review the fix for the following bug: https://bugs.openjdk.java.net/browse/JDK-8007292 This bug requires build changes and a new build tool to add additional restricted packages to the java.security file which are not part of OpenJDK. These packages are only added when doing a build including the open and closed sources. The restricted packages and new test are in the closed sources and will be reviewed separately. webrev: http://cr.openjdk.java.net/~mullan/webrevs/8007292/webrev.00/ Based on your description and the ifndef OPENJDK it sounds to me that this doesn't belong in the OpenJDK makefiles. I agree in principle, but the pattern on how to handle this isn't well established yet and something we need to improve on. In this case we would need to make changes on the open side anyway to provide hooks for overrides of the behavior. I'm willing to accept this for now. Thanks. Also, keep in mind that this hook allows each vendor,etc to add additional proprietary or internal packages to the restricted packages properties for their own build. So I think it is useful in general in that respect. That aside I would think the CP+RM could be changed to a MV. Agreed. Right. Will do. I would prefer the TOOL_ADDTO... line to be moved to jdk/makefiles/Tools.gmk with the other similar definitions, even if it is only used here atm. Ok. In the tool this code doesn't show correct use of try-with-resources: 51 try (BufferedReader br = new BufferedReader(new FileReader(args[0])); 52 BufferedWriter bw = new BufferedWriter(new FileWriter(args[1]))) { The FileReader and FileWriter should also be covered by TWR: try (FileReader fr = new FileReader(args[0]); BufferedReader br = new BufferedReader(fr); FileWriter fw = new FileWriter(args[1]); BufferedWriter bw = new BufferedWriter(fw)) { I'm not familiar with the try-with-resources, but calling close on a BufferedReader/writer will close the underlying reader/writer so nothing will be left open, will it not? That's what I thought as well. David? Finally do we still use make/tools/Makefile in the new build? No, we don't. If you want to add old build support for this, you would also need to add usage of the tool to the correct old makefile. I don't think it's necessary to add this to the old build at this point. I'll post another webrev later in the day with these updates. Thanks, Sean
hg: jdk8/tl/jdk: 8023524: Mechanism to dump generated lambda classes / log lambda code generation
Changeset: c070001c4f60 Author:henryjen Date: 2013-10-09 09:41 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c070001c4f60 8023524: Mechanism to dump generated lambda classes / log lambda code generation Reviewed-by: plevart, mchung, forax, jjb Contributed-by: brian.go...@oracle.com, henry@oracle.com ! src/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java + src/share/classes/java/lang/invoke/ProxyClassesDumper.java + test/java/lang/invoke/lambda/LogGeneratedClassesTest.java
Re: Creating an EC Public Key using Named Curves
On 2013-10-08 17:41, Vincent Ryan wrote: > Currently, there is no public API for named curves. Since I wanted a source-compatible BC, JDK 6-7, and Android solution, I ended-up using public key samples instead: ECParameterSpec spec = ((ECPublicKey) KeyFactory.getInstance ("EC").generatePublic (new X509EncodedKeySpec (sample_public_key))).getParams (); https://code.google.com/p/openkeystore/source/browse/library/trunk/src/org/webpki/crypto/KeyAlgorithms.java It is not pretty but since it is one-time op I can (probably) live with it. Anders > > However you can generate named curves using the SunEC provider and the > ECParameterSpec class. > For example, > > AlgorithmParameters parameters = > AlgorithmParameters.getInstance("EC", "SunEC"); > parameters.init(new ECGenParameterSpec("secp256r1")); > ECParameterSpec ecParameters = > parameters.getParameterSpec(ECParameterSpec.class); > > return KeyFactory.getInstance("EC", "SunEC").generatePublic(new > ECPublicKeySpec(new ECPoint(x, y), ecParameters)); > > > It's not elegant but the list of supported named curves can be extracted from > the AlgorithmParameters.EC SupportedCurves > property. For example, > > String[] curves = Security.getProvider("SunEC") > .getProperty("AlgorithmParameters.EC SupportedCurves") > .split("\\|"); > for (String curve : curves) { > System.out.println(curve.substring(1, curve.indexOf(","))); > } > > > > > On 8 Oct 2013, at 13:53, Anders Rundgren wrote: > >> If you have the X and Y points and the name of a public key you can create a >> ECPublicKey using BouncyCastle. >> I cannot find any counterpart in JDK 7. What am I missing? >> >> BC: >> >> return KeyFactory.getInstance ("EC").generatePublic (new ECPublicKeySpec >> (new ECPoint (x, y), new ECNamedCurveSpec (name,...))); >> >> Cheers >> Anders >
hg: jdk8/tl/jdk: 2 new changesets
Changeset: e3b70e601c1c Author:rriggs Date: 2013-10-09 11:02 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e3b70e601c1c 8024612: java/time/tck/java/time/format/TCKDateTimeFormatters.java failed Summary: The test should be using the Locale.Category.FORMAT to verify the test data Reviewed-by: lancea ! test/java/time/tck/java/time/format/TCKDateTimeFormatters.java Changeset: 09e2a73aa1dc Author:rriggs Date: 2013-09-26 15:19 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/09e2a73aa1dc 8025718: Enhance error messages for parsing Summary: Add values and types to exception messages Reviewed-by: lancea Contributed-by: scolebou...@joda.org ! src/share/classes/java/time/DayOfWeek.java ! src/share/classes/java/time/Instant.java ! src/share/classes/java/time/LocalDate.java ! src/share/classes/java/time/LocalDateTime.java ! src/share/classes/java/time/LocalTime.java ! src/share/classes/java/time/Month.java ! src/share/classes/java/time/MonthDay.java ! src/share/classes/java/time/OffsetDateTime.java ! src/share/classes/java/time/OffsetTime.java ! src/share/classes/java/time/Year.java ! src/share/classes/java/time/YearMonth.java ! src/share/classes/java/time/ZoneId.java ! src/share/classes/java/time/ZoneOffset.java ! src/share/classes/java/time/ZonedDateTime.java ! src/share/classes/java/time/format/Parsed.java ! test/java/time/test/java/time/format/TestDateTimeFormatter.java
hg: jdk8/tl/jdk: 8026027: Level.parse should return the custom Level instance instead of the mirrored Level
Changeset: cf6e39cfdf50 Author:mchung Date: 2013-10-09 06:24 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cf6e39cfdf50 8026027: Level.parse should return the custom Level instance instead of the mirrored Level Reviewed-by: dfuchs, chegar ! src/share/classes/java/util/logging/Level.java + test/java/util/logging/Level/CustomLevel.java + test/java/util/logging/Level/myresource.properties
hg: jdk8/tl/jdk: 8020061: Clarify reporting characteristics between splits
Changeset: 1cd20806fd5c Author:psandoz Date: 2013-10-09 15:19 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1cd20806fd5c 8020061: Clarify reporting characteristics between splits Reviewed-by: alanb, chegar ! src/share/classes/java/util/Spliterator.java
Re: [8] Review Request for 8007292 : Add JavaFX internal packages to package.access
On 10/09/2013 05:14 AM, Erik Joelsson wrote: Hello Sean, On 2013-10-09 06:33, David Holmes wrote: Hi Sean, Not a full review. On 9/10/2013 5:52 AM, Sean Mullan wrote: Please review the fix for the following bug: https://bugs.openjdk.java.net/browse/JDK-8007292 This bug requires build changes and a new build tool to add additional restricted packages to the java.security file which are not part of OpenJDK. These packages are only added when doing a build including the open and closed sources. The restricted packages and new test are in the closed sources and will be reviewed separately. webrev: http://cr.openjdk.java.net/~mullan/webrevs/8007292/webrev.00/ Based on your description and the ifndef OPENJDK it sounds to me that this doesn't belong in the OpenJDK makefiles. I agree in principle, but the pattern on how to handle this isn't well established yet and something we need to improve on. In this case we would need to make changes on the open side anyway to provide hooks for overrides of the behavior. I'm willing to accept this for now. Thanks. Also, keep in mind that this hook allows each vendor,etc to add additional proprietary or internal packages to the restricted packages properties for their own build. So I think it is useful in general in that respect. That aside I would think the CP+RM could be changed to a MV. Agreed. Right. Will do. I would prefer the TOOL_ADDTO... line to be moved to jdk/makefiles/Tools.gmk with the other similar definitions, even if it is only used here atm. Ok. In the tool this code doesn't show correct use of try-with-resources: 51 try (BufferedReader br = new BufferedReader(new FileReader(args[0])); 52 BufferedWriter bw = new BufferedWriter(new FileWriter(args[1]))) { The FileReader and FileWriter should also be covered by TWR: try (FileReader fr = new FileReader(args[0]); BufferedReader br = new BufferedReader(fr); FileWriter fw = new FileWriter(args[1]); BufferedWriter bw = new BufferedWriter(fw)) { I'm not familiar with the try-with-resources, but calling close on a BufferedReader/writer will close the underlying reader/writer so nothing will be left open, will it not? That's what I thought as well. David? Finally do we still use make/tools/Makefile in the new build? No, we don't. If you want to add old build support for this, you would also need to add usage of the tool to the correct old makefile. I don't think it's necessary to add this to the old build at this point. I'll post another webrev later in the day with these updates. Thanks, Sean
hg: jdk8/tl/nashorn: 3 new changesets
Changeset: 8d29733ad609 Author:sundar Date: 2013-10-09 10:47 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/8d29733ad609 8026112: Function("with(x ? 1e81 : (x2.constructor = 0.1)){}") throws AssertionError: double is not compatible with object Reviewed-by: lagergren, hannesw ! src/jdk/nashorn/internal/codegen/CodeGenerator.java + test/script/basic/JDK-8026112.js Changeset: 1e03d7caa68b Author:sundar Date: 2013-10-09 13:26 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/1e03d7caa68b 8026125: Array.prototype.slice.call(Java.type("java.util.HashMap")) throws ClassCastException: jdk.internal.dynalink.beans.StaticClass cannot be cast to jdk.nashorn.internal.runtime.ScriptObject Reviewed-by: hannesw, jlaskey ! src/jdk/nashorn/internal/objects/NativeArray.java + test/script/basic/JDK-8026125.js Changeset: ec3094d9d5d5 Author:hannesw Date: 2013-10-09 14:50 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/ec3094d9d5d5 8026008: Constant folding removes var statement Reviewed-by: sundar, jlaskey ! src/jdk/nashorn/internal/codegen/FoldConstants.java + test/script/basic/JDK-8026008.js + test/script/basic/JDK-8026008.js.EXPECTED
hg: jdk8/tl/langtools: 2 new changesets
Changeset: 0be3f1820e8b Author:jlahoda Date: 2013-10-09 13:06 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/0be3f1820e8b 8025141: java.lang.ClassFormatError: Illegal field modifiers in class (...) Summary: Should not generate non-public $assertionsDisabled field into interfaces Reviewed-by: jjg, vromero ! src/share/classes/com/sun/tools/javac/comp/Lower.java + test/tools/javac/defaultMethods/Assertions.java + test/tools/javac/defaultMethods/CannotChangeAssertionsStateAfterInitialized.java Changeset: 1b469fd31e35 Author:jlahoda Date: 2013-10-09 13:09 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/1b469fd31e35 8025087: Annotation processing api returns default modifier for interface static method Summary: ClassReader must not set Flags.DEFAULT for interface static methods Reviewed-by: vromero, jjg ! make/build.xml ! src/share/classes/com/sun/tools/javac/jvm/ClassReader.java ! src/share/classes/com/sun/tools/javac/resources/compiler.properties + test/tools/javac/defaultMethods/BadClassfile.java ! test/tools/javac/diags/examples.not-yet.txt + test/tools/javac/diags/examples/InvalidDefaultInterface/InvalidDefaultInterface.java + test/tools/javac/diags/examples/InvalidDefaultInterface/processors/CreateBadClassFile.java + test/tools/javac/diags/examples/InvalidStaticInterface/InvalidStaticInterface.java + test/tools/javac/diags/examples/InvalidStaticInterface/processors/CreateBadClassFile.java ! test/tools/javac/processing/model/element/TestExecutableElement.java
hg: jdk8/tl/jdk: 8008171: Refactor KeyStore.DomainLoadStoreParameter as a standalone class
Changeset: 91a752e3d50b Author:vinnie Date: 2013-10-09 10:48 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/91a752e3d50b 8008171: Refactor KeyStore.DomainLoadStoreParameter as a standalone class Reviewed-by: mullan, weijun + src/share/classes/java/security/DomainLoadStoreParameter.java ! src/share/classes/java/security/KeyStore.java ! src/share/classes/sun/security/provider/DomainKeyStore.java ! test/sun/security/provider/KeyStore/DKSTest.java
Re: [8] Review Request for 8007292 : Add JavaFX internal packages to package.access
Hello Sean, On 2013-10-09 06:33, David Holmes wrote: Hi Sean, Not a full review. On 9/10/2013 5:52 AM, Sean Mullan wrote: Please review the fix for the following bug: https://bugs.openjdk.java.net/browse/JDK-8007292 This bug requires build changes and a new build tool to add additional restricted packages to the java.security file which are not part of OpenJDK. These packages are only added when doing a build including the open and closed sources. The restricted packages and new test are in the closed sources and will be reviewed separately. webrev: http://cr.openjdk.java.net/~mullan/webrevs/8007292/webrev.00/ Based on your description and the ifndef OPENJDK it sounds to me that this doesn't belong in the OpenJDK makefiles. I agree in principle, but the pattern on how to handle this isn't well established yet and something we need to improve on. In this case we would need to make changes on the open side anyway to provide hooks for overrides of the behavior. I'm willing to accept this for now. That aside I would think the CP+RM could be changed to a MV. Agreed. I would prefer the TOOL_ADDTO... line to be moved to jdk/makefiles/Tools.gmk with the other similar definitions, even if it is only used here atm. In the tool this code doesn't show correct use of try-with-resources: 51 try (BufferedReader br = new BufferedReader(new FileReader(args[0])); 52 BufferedWriter bw = new BufferedWriter(new FileWriter(args[1]))) { The FileReader and FileWriter should also be covered by TWR: try (FileReader fr = new FileReader(args[0]); BufferedReader br = new BufferedReader(fr); FileWriter fw = new FileWriter(args[1]); BufferedWriter bw = new BufferedWriter(fw)) { I'm not familiar with the try-with-resources, but calling close on a BufferedReader/writer will close the underlying reader/writer so nothing will be left open, will it not? Finally do we still use make/tools/Makefile in the new build? No, we don't. If you want to add old build support for this, you would also need to add usage of the tool to the correct old makefile. /Erik
Re: [8] 8008171: Refactor KeyStore.DomainLoadStoreParameter as a standalone class
Thanks Max. On 9 Oct 2013, at 01:51, Weijun Wang wrote: > The code change looks fine. Do we need a CCC for it? Yes. CCC has been approved. > > Thanks > Max > > p.s. I would use the bugs.openjdk.java.net URL. Right. My webrev generates the old URL by default. I've changed it to use OpenJDK in future. > > On 10/9/13 2:14 AM, Vincent Ryan wrote: >> Please review the following change - it's a simple re-factoring to promote a >> nested class to a stand-alone class: >> >> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8008171 >> Webrev: http://cr.openjdk.java.net/~vinnie/8008171/webrev.00/ >> >> Thanks. >>
hg: jdk8/tl/jdk: 8008662: Add @jdk.Exported to JDK-specific/exported APIs
Changeset: 2ea162b2ff55 Author:alanb Date: 2013-10-09 09:20 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2ea162b2ff55 8008662: Add @jdk.Exported to JDK-specific/exported APIs Reviewed-by: chegar, vinnie, dfuchs, mchung, mullan, darcy ! src/share/classes/com/sun/jdi/AbsentInformationException.java ! src/share/classes/com/sun/jdi/Accessible.java ! src/share/classes/com/sun/jdi/ArrayReference.java ! src/share/classes/com/sun/jdi/ArrayType.java ! src/share/classes/com/sun/jdi/BooleanType.java ! src/share/classes/com/sun/jdi/BooleanValue.java ! src/share/classes/com/sun/jdi/Bootstrap.java ! src/share/classes/com/sun/jdi/ByteType.java ! src/share/classes/com/sun/jdi/ByteValue.java ! src/share/classes/com/sun/jdi/CharType.java ! src/share/classes/com/sun/jdi/CharValue.java ! src/share/classes/com/sun/jdi/ClassLoaderReference.java ! src/share/classes/com/sun/jdi/ClassNotLoadedException.java ! src/share/classes/com/sun/jdi/ClassNotPreparedException.java ! src/share/classes/com/sun/jdi/ClassObjectReference.java ! src/share/classes/com/sun/jdi/ClassType.java ! src/share/classes/com/sun/jdi/DoubleType.java ! src/share/classes/com/sun/jdi/DoubleValue.java ! src/share/classes/com/sun/jdi/Field.java ! src/share/classes/com/sun/jdi/FloatType.java ! src/share/classes/com/sun/jdi/FloatValue.java ! src/share/classes/com/sun/jdi/IncompatibleThreadStateException.java ! src/share/classes/com/sun/jdi/InconsistentDebugInfoException.java ! src/share/classes/com/sun/jdi/IntegerType.java ! src/share/classes/com/sun/jdi/IntegerValue.java ! src/share/classes/com/sun/jdi/InterfaceType.java ! src/share/classes/com/sun/jdi/InternalException.java ! src/share/classes/com/sun/jdi/InvalidCodeIndexException.java ! src/share/classes/com/sun/jdi/InvalidLineNumberException.java ! src/share/classes/com/sun/jdi/InvalidStackFrameException.java ! src/share/classes/com/sun/jdi/InvalidTypeException.java ! src/share/classes/com/sun/jdi/InvocationException.java ! src/share/classes/com/sun/jdi/JDIPermission.java ! src/share/classes/com/sun/jdi/LocalVariable.java ! src/share/classes/com/sun/jdi/Locatable.java ! src/share/classes/com/sun/jdi/Location.java ! src/share/classes/com/sun/jdi/LongType.java ! src/share/classes/com/sun/jdi/LongValue.java ! src/share/classes/com/sun/jdi/Method.java ! src/share/classes/com/sun/jdi/Mirror.java ! src/share/classes/com/sun/jdi/MonitorInfo.java ! src/share/classes/com/sun/jdi/NativeMethodException.java ! src/share/classes/com/sun/jdi/ObjectCollectedException.java ! src/share/classes/com/sun/jdi/ObjectReference.java ! src/share/classes/com/sun/jdi/PathSearchingVirtualMachine.java ! src/share/classes/com/sun/jdi/PrimitiveType.java ! src/share/classes/com/sun/jdi/PrimitiveValue.java ! src/share/classes/com/sun/jdi/ReferenceType.java ! src/share/classes/com/sun/jdi/ShortType.java ! src/share/classes/com/sun/jdi/ShortValue.java ! src/share/classes/com/sun/jdi/StackFrame.java ! src/share/classes/com/sun/jdi/StringReference.java ! src/share/classes/com/sun/jdi/ThreadGroupReference.java ! src/share/classes/com/sun/jdi/ThreadReference.java ! src/share/classes/com/sun/jdi/Type.java ! src/share/classes/com/sun/jdi/TypeComponent.java ! src/share/classes/com/sun/jdi/VMCannotBeModifiedException.java ! src/share/classes/com/sun/jdi/VMDisconnectedException.java ! src/share/classes/com/sun/jdi/VMMismatchException.java ! src/share/classes/com/sun/jdi/VMOutOfMemoryException.java ! src/share/classes/com/sun/jdi/Value.java ! src/share/classes/com/sun/jdi/VirtualMachine.java ! src/share/classes/com/sun/jdi/VirtualMachineManager.java ! src/share/classes/com/sun/jdi/VoidType.java ! src/share/classes/com/sun/jdi/VoidValue.java ! src/share/classes/com/sun/jdi/connect/AttachingConnector.java ! src/share/classes/com/sun/jdi/connect/Connector.java ! src/share/classes/com/sun/jdi/connect/IllegalConnectorArgumentsException.java ! src/share/classes/com/sun/jdi/connect/LaunchingConnector.java ! src/share/classes/com/sun/jdi/connect/ListeningConnector.java ! src/share/classes/com/sun/jdi/connect/Transport.java ! src/share/classes/com/sun/jdi/connect/TransportTimeoutException.java ! src/share/classes/com/sun/jdi/connect/VMStartException.java + src/share/classes/com/sun/jdi/connect/package-info.java - src/share/classes/com/sun/jdi/connect/package.html ! src/share/classes/com/sun/jdi/connect/spi/ClosedConnectionException.java ! src/share/classes/com/sun/jdi/connect/spi/Connection.java ! src/share/classes/com/sun/jdi/connect/spi/TransportService.java + src/share/classes/com/sun/jdi/connect/spi/package-info.java - src/share/classes/com/sun/jdi/connect/spi/package.html ! src/share/classes/com/sun/jdi/event/AccessWatchpointEvent.java ! src/share/classes/com/sun/jdi/event/BreakpointEvent.java ! src/share/classes/com/sun/jdi/event/ClassPrepareEvent.java ! src/share/classes/com/sun/jdi/event/ClassUnloadEvent.java ! src/share/classes/com/sun/jdi/event/Event.java ! src/share/classes/com/sun/jdi/event/EventIterator.java ! src/share/classes/
Re: [8] 8008171: Refactor KeyStore.DomainLoadStoreParameter as a standalone class
Thanks. On 8 Oct 2013, at 19:33, Sean Mullan wrote: > Looks good to me. > > --Sean > > On 10/08/2013 02:14 PM, Vincent Ryan wrote: >> Please review the following change - it's a simple re-factoring to promote a >> nested class to a stand-alone class: >> >> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8008171 >> Webrev: http://cr.openjdk.java.net/~vinnie/8008171/webrev.00/ >> >> Thanks. >> >
hg: jdk8/tl/langtools: 8024415: Bug in javac Pretty: Wrong precedence in JCConditional trees
Changeset: ea000904db62 Author:alundblad Date: 2013-10-08 15:33 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/ea000904db62 8024415: Bug in javac Pretty: Wrong precedence in JCConditional trees Summary: Fixed precedence and associativity issues with pretty printing of JCConditional expressions. Reviewed-by: jfranck Contributed-by: Andreas Lundblad , Matthew Dempsky ! src/share/classes/com/sun/tools/javac/tree/Pretty.java + test/tools/javac/tree/T8024415.java