-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support big endian #13
Conversation
src/lexer.js
Outdated
let i = 0; | ||
while (i < len) { | ||
outBuf16[i] = src.charCodeAt(i * 2 + 1); | ||
outBuf16[i + 1] = src.charCodeAt(i * 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't appear to work as
outBuf16[i + 1]
gets overwritten the next time around the loop byoutBuf16[i]
- it swaps pairs of 16-bit values rather than the octets within the 16-bit value.
I've hacked something together that appears to work on AIX (tests pass). I'll need to tidy up a bit (I've almost certainly broken little endian in what I currently have) -- I'll try to get a PR in either later tonight or tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @richardlau, I've pushed up some fixes here, running on nodejs/node#35634 to see if it works, but if it's still failing your PR would be a huge help.
Finally got it working! |
let i = 0; | ||
while (i < len) { | ||
const ch = src.charCodeAt(i); | ||
outBuf16[i++] = (ch & 0xff) << 8 | (ch & 0xff0) >> 8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is missing a 0
, which causes the most significant nibble to be discared:
outBuf16[i++] = (ch & 0xff) << 8 | (ch & 0xff0) >> 8; | |
outBuf16[i++] = (ch & 0xff) << 8 | (ch & 0xff00) >> 8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for spotting this, it wasn't causing an issue because the right shift drops these bits anyway, the version on master has the fix.
This adds big endian support to the copy function.
I don't want to merge this until it has actually been tested on the right hardware, but don't have access to a test machine - any assitance testing would be really great.