Fix operator precedence error with signed integer reading.

[jchv]

TypeScript caught that these branches were always false... because of operator precedence.

I have not checked to see if this fixes an actual bug or not. It may be worth adding a test case for this first, if it actually does.

[GreyCat]

Wow! Thanks for this gem. This definitely warrants extra investigation and a test case.

[jchv]

Bump on this one. Should I find a way to test it?

[cherue]

I investigated this issue and here is what I found:

Rewriting the Expression

For reference the function currently looks like this:

KaitaiStream.prototype.readS8be = function(e) {
  var v1 = this.readU4be();
  var v2 = this.readU4be();

  if (v1 & 0x80000000 != 0) {
    // negative number
    return -(0x100000000 * (v1 ^ 0xffffffff) + (v2 ^ 0xffffffff)) - 1;
  } else {
    return 0x100000000 * v1 + v2;
  }
};

The expression (v1 & 0x80000000 != 0) can be grouped to (v1 & (0x80000000 != 0)) because the != unequal operator has precedence over the & bitwise-and operator.

Since (0x80000000 != 0) is always true it can be rewritten to (v1 & true). JS treats true like 0x01 in this context so it can further be rewritten to (v1 & 0x01).

We started with (v1 & 0x80000000 != 0) and now have (v1 & 0x01).

Why it kindof works right now

What currently happens is the whole number is treated as negative if the high bytes (v1 for readS8be) are an odd number (have their lowest bit set). This translates to ranges of 2^32 numbers that are correctly treated as negative followed by 2^32 numbers that are incorrectly treated as positive.

negative:
-1              0x ff ff ff ff ff ff ff ff
-(2^32)         0x ff ff ff ff 00 00 00 00
                             ^
positive:
-(2^32) -1      0x ff ff ff fe ff ff ff ff
-(2^32) -(2^32) 0x ff ff ff fe 00 00 00 00
                             ^
and so on

Test

There isn't a general test that would have caught this. I checked all the other runtimes and none of them do it this way - this is a JS specific hack. TypeScript would have helped here.

Although I did rewrite the integers.kst to check min and max values of both signed and unsigned integers which would have caught this, but only on accident. I'll make a PR once I figure out how KST works.

Conclusion

We should adopt TypeScript! Also this should be merged.

[GreyCat]

Thanks for such an extensive investigation and explanations! Merging this in :)