Andrew,
Thanks for working on it.
The change looks fine, except one final nit:-)
EUC_JP_Open: #127 & #140
it appears encoderJ0208 is no longer needed.
Please run those tests at sun/nio/cs before push, I don't really trust
my own eyes
these days:-)
Sherman
Andrew John Hughes wrote:
On 9 June 2010 18:22, Xueming Shen <xueming.s...@oracle.com> wrote:
Andrew John Hughes wrote:
On 9 June 2010 07:55, Florian Weimer <fwei...@bfk.de> wrote:
* Andrew John Hughes:
On 7 June 2010 23:04, Xueming Shen <xueming.s...@oracle.com> wrote:
Hi Andrew,
6959197: When building with JAVAC_MAX_WARNINGS=true, the build fails in
sun/nio/cs due to the use of -Werror
(1)sun/io/ByteTocharISO2022JP.java
#129, #151
if ((byte1 & (byte)0x80) != 0){
if ((byte2 & (byte)0x80) != 0){
(byte) casting is not necessary as well?
It's necessary. 0x80 is an integer literal
(http://java.sun.com/docs/books/jls/third_edition/html/lexical.html#3.10.1)
which requires a lossy narrowing conversion to byte
(http://java.sun.com/docs/books/jls/third_edition/html/conversions.html#5.1.3)
Doesn't the & operator promote its arguments to int anyway? I don't
think the cast to byte makes a difference here because it does not
matter if 0x80 is sign-extended or zero-extended.
It might be more efficient to use "byte1 < 0" and "byte2 < 0" instead.
You're right. I think I mentally read that as an assignment, not an
and operation.
If I'm reading it right now, it would seem to just be testing the sign
bit. byte1 < 0 would be clearer if not faster, and I'd be for
changing that, though that probably should be a separate patch.
Sherman, does the new webrev look ok to push?
(1)ByteToCharJISAutoDetect.java
It appears we should move the maskTable1/2 initialization code out of the
constructor block.
37 private static byte[] maskTable1 = JISAutoDetect.getByteMask1();
38 private static byte[] maskTable2 = JISAutoDetect.getByteMask2();
Done. I also made them final.
(2)EUC_JP_Open.java
137 j0208Index1 = JIS_X_0208_Solaris_Encoder.getIndex1();
138 j0208Index2 = JIS_X_0208_Solaris_Encoder.getIndex2();
can be moved out of the constructor as you do for the decoder. better to
keep them consistent (as well as the code in EUC_JP_LINUX)
Done. I made both sets private, static and final as well, and updated
EUC_JP, EUC_JP_LINUX, PCK and SJIS in the same way.
(3)sun/nio/cs/ext/HKSCS.java
- this.big5Dec = big5Dec;
+ Decoder.big5Dec = big5Dec;
Dave is absolutely right here. big5Dec definitely should be an instance
field instead of static, it's my bug in the original code. It should be
private DoubleByte.Decoder big5Dec;
protected Decoder(Charset cs,
DoubleByte.Decoder big5Dec,
char[][] b2cBmp, char[][] b2cSupp)
{
// super(cs, 0.5f, 1.0f);
// need to extends DoubleByte.Decoder so the
// sun.io can use it. this implementation
super(cs, 0.5f, 1.0f, null, null, 0, 0);
this.big5Dec = big5Dec;
this.b2cBmp = b2cBmp;
this.b2cSupp = b2cSupp;
}
Cool. One latent bug fixed too :-)
Thanks,
Sherman
New webrev: http://cr.openjdk.java.net/~andrew/warnings/webrev.05/
Ok to push?
Thanks,
