PING: Re: Fix build failure with JAVAC_MAX_WARNINGS=true in sun/nio/cs

[Xueming Shen]

Andrew John Hughes wrote:
> On 9 June 2010 07:55, Florian Weimer <fweimer at bfk.de> wrote:
>   
>> * Andrew John Hughes:
>>
>>     
>>> On 7 June 2010 23:04, Xueming Shen <xueming.shen at 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();

(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)


(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;
         }


Thanks,
Sherman