[ http://issues.apache.org/jira/browse/IO-101?page=comments#action_12456131 
] 
            
Robert Michel commented on IO-101:
----------------------------------

Henri,

I came across this bug just yesterday and saw your patch today.  Separately, I 
had come up with an alternate fix which you might want to consider.

The older code fails whenever the byte at offset+3 is negative.  The int isn't 
wrapping, but it is extending the sign when the int is cast up to long.  The 
sign extension is what needs to be clipped.

The byte at offset+7 is not affected, because the variable it enters (high) is 
shifted left 32, obliterating the sign bit.

The current solution converts the byte at offset+3 to a long, which fixes this 
but will silently upcast the other bytes to long to accomplish the additions.  
I suspect the original author had intended to do all the swapping in ints for 
performance reasons.   (If performance wasn't an issue, this method could be 
done in a single line.)  I'm guessing that the casts to long and the addition 
on the last line shoud be the only 64-bit operations.

My alternative:

    public static long readSwappedLong(byte[] data, int offset) {
        int low = (
            ( ( data[ offset + 0 ] & 0xff ) << 0 ) +
            ( ( data[ offset + 1 ] & 0xff ) << 8 ) +
            ( ( data[ offset + 2 ] & 0xff ) << 16 ) +
            ( ( data[ offset + 3 ] & 0xff ) << 24 ) );
        int high = (
            ( ( data[ offset + 4 ] & 0xff ) << 0 ) +
            ( ( data[ offset + 5 ] & 0xff ) << 8 ) +
            ( ( data[ offset + 6 ] & 0xff ) << 16 ) +
            ( ( data[ offset + 7 ] & 0xff ) << 24 ) );
        return ((long)high << 32) + (0xffffffffL & low);
    }

This solution indicates (a little more clearly?) that the original two 
statements are 32-bit ops and the last statement is the 64-bit work. 

FYI, I used this test to demonstrate the problem locally:

    public static void main(String[] args) {
        
        byte[] buffer = {
                0, 0, 0, -1, 1, 2, 3, 4
        };
        long val = EndianUtils.readSwappedLong(buffer, 0);
        ByteBuffer bb = ByteBuffer.allocate(8);
        bb.putLong(val);
        bb.flip();
        for (int i = 0; i < 8; i++) {
            System.out.println(bb.get() + " ");
        }
        System.out.println();
        
    }

> The method EndianUtils.writeSwappedDouble() and 
> EndianUtils.readSwappedDouble() do not match!
> ---------------------------------------------------------------------------------------------
>
>                 Key: IO-101
>                 URL: http://issues.apache.org/jira/browse/IO-101
>             Project: Commons IO
>          Issue Type: Bug
>          Components: Streams/Writers
>    Affects Versions: 1.2
>         Environment: I was running Windows XP SP2 and using Commons IO 1.2, 
> Java 1.5 update 9 when I got this problem.
>            Reporter: José Pinto
>            Priority: Critical
>             Fix For: 1.3
>
>         Attachments: IO-101.patch
>
>
> Code:
> public static void main(String[] args) {
>               double[] tests = new double[] {34.345, -345.5645, 545.12, 
> 10.043, 7.123456789123};
>               for (int i = 0; i< tests.length ;i++) {
>                       byte[] buffer = new byte[8];                    
>                       EndianUtils.writeSwappedDouble(buffer, 0, tests[i]);
>                       double val = EndianUtils.readSwappedDouble(buffer, 0);
>                       System.out.println(val);        
>               }
>                
> }
> Result:
> 34.344969482421874
> -345.5645
> 545.11951171875
> 10.043
> 7.123456789123
> Note:
> In my opinion the values shouldn't be changed at all.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: 
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira



---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to