buf() mishandles [3, 0, 0, 0, 0] #9

New Issue

2017-10-15T09:14:28Z

FGasper commented

2017-10-15 09:14:28 +00:00

(Migrated from github.com)

> const CRC32_MOD = require('crc-32');
undefined
> CRC32_MOD.buf([3, 0, 0, 0, 0])
-2122150451
>

``` > const CRC32_MOD = require('crc-32'); undefined > CRC32_MOD.buf([3, 0, 0, 0, 0]) -2122150451 > ```

FGasper commented

2017-10-15 09:15:08 +00:00

(Migrated from github.com)

Never mind … I didn’t realize this computes a signed value rather than unsigned.

reviewher commented

2017-10-15 13:00:34 +00:00

(Migrated from github.com)

"The return value is a signed 32-bit integer."

See the discussion from an older issue. Basically V8 has a special optimization for 32-bit signed integers which is generally awesome but performs extremely poorly for naive CRC calculations

["The return value is a signed 32-bit integer."](https://github.com/SheetJS/js-crc32#usage) See the discussion from [an older issue](https://github.com/SheetJS/js-crc32/issues/4#issuecomment-170050796). Basically V8 has a special optimization for 32-bit signed integers which is generally awesome but performs extremely poorly for naive CRC calculations

FGasper commented

2017-10-15 13:42:58 +00:00

(Migrated from github.com)

Might be nice to include a convenience that does the signed-to-unsigned conversion?

I think it's just: 0xffffffff - ~signed?

Might be nice to include a convenience that does the signed-to-unsigned conversion? I think it's just: `0xffffffff - ~signed`?

reviewher commented

2017-10-15 13:45:20 +00:00

(Migrated from github.com)

Even easier: signed >>> 0 :) kinda seems like a waste to add that as a convenience method, but it's probably worth adding that to the README (PRs welcome)

Even easier: `signed >>> 0` :) kinda seems like a waste to add that as a convenience method, but it's probably worth adding that to the README (PRs welcome)

Sign in to join this conversation.