Re: Please review -XshowSettings a java launcher option.

On 11/11/2010 16:42, Kumar Srinivasan wrote: line 176, 188, 190-191, 195, and other lines in printPrintLocales and printLocale methods: - the assignment to the buf and out variable to itself (returned from StringBuffer.append() method) is not necessary. Yes fixed, I missed these. The "inten

Re: Please review -XshowSettings a java launcher option.

Kumar Srinivasan said the following on 11/12/10 13:11: Is the purpose here to report what command-line options were used or to report what particular settings are in effect? If the former then you do not need any defaults for the "not set" case. If the latter then you should really defer this t

Re: Please review -XshowSettings a java launcher option.

Kumar, The scaleValue(longv) does not appear to handle the 1K, 1M, 1G and 1T correctly, shouldn't be v >= K/M/G/T ? 1024 for 1K is OK, byte number for 1M/G/T might be too much. Seems like you are "assembling" the bits yourself, why not use our "powerful" (sure, a little slower:-) j.u.Forma

Re: Please review -XshowSettings a java launcher option.

Hi Mr. Holmes, Hi Kumar, Is the purpose here to report what command-line options were used or to report what particular settings are in effect? If the former then you do not need any defaults for the "not set" case. If the latter then you should really defer this to the VM itself for the "VM

Re: Please review -XshowSettings a java launcher option.

Kumar, No problem, if you have time pressures, I definitely understand. This is welcome functionality, we have our own similar implementation internally but it will be nice to have in OpenJDK. I am curious though about the concern for string concatenation on something like (INDENT + INDENT). As I

Re: Please review -XshowSettings a java launcher option.

On Nov 11 2010, at 13:56 , Kumar Srinivasan wrote: > A sample output attached below. Fantastic! - The colons on the path components seem unnecessary if each element is also going to be separated by line breaks - In the output for the property 'line.seperator' would it be possible to convert

Re: Please review -XshowSettings a java launcher option.

Hi David, Good idea, but I did not use that method because of all the String concatenation PrintStream.print has to perform, if so then we can simply use a String. So my thinking was to use the StringBuilder to assemble the String and print it out in one shot. ostream(INDENT + INDENT); or

Re: Please review -XshowSettings a java launcher option.

Hi Kumar, Is the purpose here to report what command-line options were used or to report what particular settings are in effect? If the former then you do not need any defaults for the "not set" case. If the latter then you should really defer this to the VM itself for the "VM options", becaus

Re: Please review -XshowSettings a java launcher option.

On 11/11/10 4:42 PM, Kumar Srinivasan wrote: line 1032: Perhaps you could store the suboption ("all", "vm", etc) rather than the entire option string. I want to keep all the parsing logic as much as possible in java. It depends when you are going to detect if the option is valid or not. I

Re: Please review -XshowSettings a java launcher option.

Kumar, Do you need all of the StringBuilder/StringBuffer to buffer the output prior to writing to the PrintStream? Using the PrintStream directly would also allow it to handle the newlines. I was envisaging something like this: private static void printVmSettings(PrintStream ostream, long maxHeap

Re: Please review -XshowSettings a java launcher option.

Hi Mandy, java.c line 1031: this doesn't catch invalid option if it has the -XshowSettings prefix e.g. -XshowSettingsJunk. Will fix it. line 1032: Perhaps you could store the suboption ("all", "vm", etc) rather than the entire option string. I want to keep all the parsing logic as much a

Re: Please review -XshowSettings a java launcher option.

Kumar, Great to see this functionality being added. Another useful piece of data may be the tzdata version used by the JRE. I'll open up another thread on this with you. It should be possible to get some helper code added to parse tzdata version. This could always be added at a later date of cou

Re: Please review -XshowSettings a java launcher option.

On 11/11/10 13:56, Kumar Srinivasan wrote: Hi All, Please review the following: http://cr.openjdk.java.net/~ksrini/6452854/webrev.00/ Some comments: java.c line 1031: this doesn't catch invalid option if it has the -XshowSettings prefix e.g. -XshowSettingsJunk. line 1032: Perhaps you could

Please review -XshowSettings a java launcher option.

Hi All, Please review the following: http://cr.openjdk.java.net/~ksrini/6452854/webrev.00/ This will print all the known settings/properties/locales supported and known to Java, this has been a long standing request. A sample output attached below. Note: the -X option specifically is being

Re: Please review java.util.jar.pack.* cleanup/refactoring/generificaiton

Le 11/11/2010 20:34, Kelly O'Hair a écrit : So some of these changes are using the new jdk7 language features? (diamond operator) Is this a first, or is this happening already? Just curious. It's not the first time. I have seen it time to time. Mark (Reinhold) was the first to push a patch usi

Re: Please review java.util.jar.pack.* cleanup/refactoring/generificaiton

So some of these changes are using the new jdk7 language features? (diamond operator) Is this a first, or is this happening already? Just curious. decided to use it, put NB on autopilot. :-) http://cr.openjdk.java.net/~ksrini/6990106/webrev.00/src/share/classes/com/sun/java/util/jar/pack/A

Re: Please review java.util.jar.pack.* cleanup/refactoring/generificaiton

Hi Alan, Brian, Thanks! I will look into both your comments. Kumar - I just scanned this (sorry, I don't have time to do a detailed review). - is Attribute.isEmpty right? or rather should ""==null be replaced by false? yes, I will redo this. - in BandStructure.java, can basicCodingIndexes

Re: Please review java.util.jar.pack.* cleanup/refactoring/generificaiton

So some of these changes are using the new jdk7 language features? (diamond operator) Is this a first, or is this happening already? Just curious. http://cr.openjdk.java.net/~ksrini/6990106/webrev.00/src/share/classes/com/sun/java/util/jar/pack/Attribute.java.sdiff.html 460 return

Re: Please review java.util.jar.pack.* cleanup/refactoring/generificaiton

On 11/11/2010 11:06 AM, Brian Goetz wrote: Mostly style issues, but at least one likely bug. In a few places, you've used @SuppressWarnings("unchecked"). While this may well be unavoidable, it is worth factoring this out into the smallest possible chunk. For BandStructure, you've applied it

hg: jdk7/tl/jdk: 2 new changesets

Changeset: aab6e875eb52 Author:mduigou Date: 2010-11-11 11:01 -0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/aab6e875eb52 6465367: (coll) Typo in TreeMap documentation Reviewed-by: alanb, briangoetz ! src/share/classes/java/util/TreeMap.java Changeset: ca73653c0329 Author:

Re: Please review java.util.jar.pack.* cleanup/refactoring/generificaiton

Mostly style issues, but at least one likely bug. In a few places, you've used @SuppressWarnings("unchecked"). While this may well be unavoidable, it is worth factoring this out into the smallest possible chunk. For BandStructure, you've applied it to the whole class, and there's some big me

Re: Please review java.util.jar.pack.* cleanup/refactoring/generificaiton

Kumar Srinivasan wrote: Hi, Appreciate a review of the pack200 cleanup work, in the following areas: 1. General generification of Collections. 2. fixed worthy findbugs items. 3. fixed worthy Netbeans nags. 4. Elimination of array/generics combination. The webrev is here: http://cr.openjdk.java

Please review java.util.jar.pack.* cleanup/refactoring/generificaiton

Hi, Appreciate a review of the pack200 cleanup work, in the following areas: 1. General generification of Collections. 2. fixed worthy findbugs items. 3. fixed worthy Netbeans nags. 4. Elimination of array/generics combination. The webrev is here: http://cr.openjdk.java.net/~ksrini/6990106/webr

Re: hg: jdk7/tl/jdk: 6843995: RowSet 1.1 updates

Thank you Alan. Yes I am planning on doing additional clean up across the code utilizing a separate CR. Regards, Lance On Nov 11, 2010, at 4:59 AM, Alan Bateman wrote: > Lance Andersen - Oracle wrote: >> Hi Remi (and team), >> >> I made changes to SyncFactory and one other class for a similar

Re: hg: jdk7/tl/jdk: 6843995: RowSet 1.1 updates

Lance Andersen - Oracle wrote: Hi Remi (and team), I made changes to SyncFactory and one other class for a similar error. Also cleaned up a couple of other minor issues in these classes. The webrev can be found at http://cr.openjdk.java.net/~lancea/6982530/