Re: Code Review Request for Bug #4802647

2011-12-02 Thread Alan Bateman
On 01/12/2011 22:42, Brandon Passanisi wrote: Hi Jason. Thanks for your response. I was thinking about how I can improve the test using your suggestion. I could possibly do the following: 1. Find all of the subclasses of AbstractCollection which override removeAll(Collection?) and

Re: Request for Review: Warnings cleanup in java.lang.*, java.util.**

2011-12-02 Thread Alan Bateman
cc'ing core-libs-dev as that is the place to discuss these changes. I see on the sign-up sheet [1] that omajid has signed up for java.lang, maybe you are working together? I'll leave it to Stuart to say whether he wants to refactor/other changes separated from the warnings changes. One

hg: jdk8/tl/jdk: 7116946: JSSecurityManager should use java.util.ServiceLoader to lookup service providers

2011-12-02 Thread chris . hegarty
Changeset: f615db07991e Author:chegar Date: 2011-12-02 11:39 + URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f615db07991e 7116946: JSSecurityManager should use java.util.ServiceLoader to lookup service providers Reviewed-by: prr !

Re: 7117357: Warnings in sun.instrument, tools and other sun.* classes

2011-12-02 Thread Lance Andersen - Oracle
Looks good to me Best Lance On Dec 2, 2011, at 8:42 AM, Alan Bateman wrote: I'm down on the sign-up sheet for warnings in a couple of arcane areas. The following webrev fixes the warnings in sun.instrument.*, several serviceability tools, serialver, and some residual warnings left in

Re: 7116957: javax.script.ScriptEngineManager should use java.util.ServiceLoader to lookup service providers

2011-12-02 Thread Alan Bateman
On 02/12/2011 11:30, Chris Hegarty wrote: Alan, Would you mind taking a quick look at the updated webrev? Trivially, I fixed a few style issues in HttpServerProvider (a nit of mine) and updated the commented code in FtpClientProvider (in case it ever gets enabled ). Let's remove any

Re: 7117357: Warnings in sun.instrument, tools and other sun.* classes

2011-12-02 Thread Chris Hegarty
On 02/12/2011 13:49, Lance Andersen - Oracle wrote: Looks good to me Thumbs up from me too. -Chris. Best Lance On Dec 2, 2011, at 8:42 AM, Alan Bateman wrote: I'm down on the sign-up sheet for warnings in a couple of arcane areas. The following webrev fixes the warnings in

Request for Review (XXL): 7104647: Adding a diagnostic command framework

2011-12-02 Thread Frederic Parain
Hi All, [Posting to serviceability-dev, runtime-dev and core-libs-dev because changes are pretty big and touch all these areas] Here's a framework for issuing diagnostics commands to the JVM. Diagnostic commands are actions executed inside the JVM mainly for monitoring or management purpose.

Cleanup fallthrough in FilePermission and PropertyPermission was: Request for Review of 7116890 (Warning Cleanup java.io)

2011-12-02 Thread Sebastian Sickelmann
Hi Brandon, i will try to work out a fix for both and cc the review request to you. -- Sebastian Am 01.12.2011 23:54, schrieb Brandon Passanisi: Hi Sebastian. I was speaking with Stuart Marks earlier today and he mentioned that the fallthrough code in FilePermission.java also exists in

Re: Request for Review of 7116890 (Warning Cleanup java.io)

2011-12-02 Thread Sebastian Sickelmann
Am 02.12.2011 01:22, schrieb Stuart Marks: On 12/1/11 2:13 PM, Stuart Marks wrote: On 12/1/11 12:38 PM, Alan Bateman wrote: On 01/12/2011 18:35, Sebastian Sickelmann wrote: : Thanks Alan, L67-68 was a backporting (from a more complex solution to a small warning cleanup) issue. I missed the

Re: 7116445: Miscellaneous warnings in the JDBC/RowSet classes

2011-12-02 Thread Lance Andersen - Oracle
Here is the diff for DriverManager, I won't be pushing another webrev unless the word is to go ahead and add @Deprecated to the com/* classes of the RowSet RI or there is another change requested that is more detailed: dhcp-adc-twvpn-2-vpnpool-10-154-44-9:sql lanceandersen$ hg diff

RFR 7117360: Warnings in java.util.concurrent.atomic package

2011-12-02 Thread Chris Hegarty
Cleanup compiler warnings in the java.util.concurrent.atomic package. This is a sync up with the raw type warning fixes in Doug's CVS, along with some minor style cleanup. With this change there are still 2 outstanding unchecked casts in AtomicReferenceArray and AtomicReferenceFieldUpdater.

Re: RFR 7117360: Warnings in java.util.concurrent.atomic package

2011-12-02 Thread Chris Hegarty
Oh, AtomicBoolean.java and AtomicReference.java show no differences in the webrev, but there is a correction to the indentation in the static initializer. And just to point out, along with raw type warning fixes there is a removal of a redundant cast ;-) -Chris. On 02/12/2011 14:25,

Re: RFR 7117360: Warnings in java.util.concurrent.atomic package

2011-12-02 Thread Doug Lea
On 12/02/11 09:25, Chris Hegarty wrote: Cleanup compiler warnings in the java.util.concurrent.atomic package. This is a sync up with the raw type warning fixes in Doug's CVS, along with some minor style cleanup. With this change there are still 2 outstanding unchecked casts in

Re: Cleanup fallthrough in FilePermission and PropertyPermission was: Request for Review of 7116890 (Warning Cleanup java.io)

2011-12-02 Thread Sebastian Sickelmann
Am 02.12.2011 16:27, schrieb Brandon Passanisi: Hi Sebastian. I'm not sure if you had seen the e-mail from Stuart Marks regarding this, but Stuart was able to find more instances of the similar block of fallthrough code. I can volunteer to apply your upcoming change to FilePermission to the

Re: RFR 7117360: Warnings in java.util.concurrent.atomic package

2011-12-02 Thread Chris Hegarty
On 12/ 2/11 04:22 PM, Doug Lea wrote: We just went through these and others, and believe that everything is now warning free. Thanks Doug, Wow you guys are quick! I pulled in these specific changes (atomic) and updated the webrev:

hg: jdk8/tl/jdk: 7117357: Warnings in sun.instrument, tools and other sun.* classes

2011-12-02 Thread alan . bateman
Changeset: 9950e2c9f3b5 Author:alanb Date: 2011-12-02 17:37 + URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/9950e2c9f3b5 7117357: Warnings in sun.instrument, tools and other sun.* classes Reviewed-by: lancea, chegar ! src/share/classes/sun/instrument/InstrumentationImpl.java

Re: Request for Review: 7116914 (Miscellaneous warnings (sun.text))

2011-12-02 Thread Mike Duigou
Looks good. In NormalizerImpl.java: - The parens are probably not needed around 'length=(srcIndex-prevSrc);' UnicodeSet.java: - I confirmed, just to be sure, that the added 'break;' statements have no effect. On Dec 1 2011, at 23:51 , Yuka Kamiya wrote: Hello, Could someone please review

Re: 7116445: Miscellaneous warnings in the JDBC/RowSet classes

2011-12-02 Thread Stuart Marks
Hi Lance, I'm OK with postponing the @Deprecated work, and doing a separate pass for @Deprecated, including the com.sun.* stuff at that time. There's enough stuff in this changeset already. I think we're better off getting it in now than putting more stuff in and getting it reviewed again. I

Re: Request for Review: 7116914 (Miscellaneous warnings (sun.text))

2011-12-02 Thread Stuart Marks
On 12/1/11 11:51 PM, Yuka Kamiya wrote: Hello, Could someone please review this fix? http://cr.openjdk.java.net/~peytoia/7116914/webrev.00/ Some warnings are still issued even after this fix, and that's intentional. I'd like to solve them in another way rather than using

Re: Cleanup fallthrough in FilePermission and PropertyPermission was: Request for Review of 7116890 (Warning Cleanup java.io)

2011-12-02 Thread Stuart Marks
I'm adding Weijun (Max) Wang to this thread. The same ackbarfaccept code had come up a third time when I was reviewing some of Max's changes. The code in question all has to do with permissions, and Max is in the security group, so he might have a better insight whether doing a refactoring is

Re: Request for Review: 7116914 (Miscellaneous warnings (sun.text))

2011-12-02 Thread Stuart Marks
On 12/2/11 12:25 PM, Mike Duigou wrote: UnicodeSet.java: - I confirmed, just to be sure, that the added 'break;' statements have no effect. A stylistic comment, not directly relevant to warnings fixes. The compiler warns about possible fall-through to the next case, but the locations where

Re: 7116445: Miscellaneous warnings in the JDBC/RowSet classes

2011-12-02 Thread Lance Andersen - Oracle
Hi Stuart, On Dec 2, 2011, at 3:45 PM, Stuart Marks wrote: Hi Lance, I'm OK with postponing the @Deprecated work, and doing a separate pass for @Deprecated, including the com.sun.* stuff at that time. OK, will do that separately There's enough stuff in this changeset already. I think

Review request for bug #5063455

2011-12-02 Thread Brandon Passanisi
Hello core-libs-dev. I was wondering if somebody could review the following proposed fix and test case for bug #5063455. Here's the info: Bug URL: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5063455 Webrev: http://cr.openjdk.java.net/~sherman/5063455/webrev/ Thanks. -- Oracle

Re: Review Request for 7116997 (warning cleanup java.util.PropertyPermission)

2011-12-02 Thread Brandon Passanisi
Thanks for the review Stuart. Here is a another webrev for review containing your suggested change: http://cr.openjdk.java.net/~dmocek/bpassanisi/7116997/webrev.01/ http://cr.openjdk.java.net/%7Edmocek/bpassanisi/7116997/webrev.01/ On 12/1/2011 4:06 PM, Stuart Marks wrote: On 12/1/11

hg: jdk8/tl/jdk: 7117465: Warning cleanup for IMF classes

2011-12-02 Thread naoto . sato
Changeset: 42532a097816 Author:naoto Date: 2011-12-02 16:04 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/42532a097816 7117465: Warning cleanup for IMF classes Reviewed-by: okutsu ! src/share/classes/java/awt/im/InputMethodHighlight.java !

hg: jdk8/tl/jdk: 5035850: (str) String.CASE_INSENSITIVE_ORDER should override readResolve()

2011-12-02 Thread xueming . shen
Changeset: 1d7037df65ed Author:sherman Date: 2011-12-02 16:25 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1d7037df65ed 5035850: (str) String.CASE_INSENSITIVE_ORDER should override readResolve() Summary: Fix to ensure singleton property of String.CaseInsensitiveComparator

hg: jdk8/tl/jdk: 7117585: Eliminate java.lang.instrument, java.lang.management warnings

2011-12-02 Thread mandy . chung
Changeset: 98502d7a3f98 Author:mchung Date: 2011-12-02 16:29 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/98502d7a3f98 7117585: Eliminate java.lang.instrument, java.lang.management warnings Reviewed-by: mchung Contributed-by: Jon VanAlten jon.vanal...@redhat.com !

hg: jdk8/tl/jdk: 7117487: Warnings Cleanup: some i18n classes in java.util and sun.util

2011-12-02 Thread masayoshi . okutsu
Changeset: f2a5d0001f15 Author:okutsu Date: 2011-12-03 10:58 +0900 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f2a5d0001f15 7117487: Warnings Cleanup: some i18n classes in java.util and sun.util Reviewed-by: lancea, naoto ! src/share/classes/java/util/Date.java !