search

alexgorbatchev / crc

Blazingly fast CRC implementations for node.js and browser
MIT License
349 stars 72 forks source link

wrong return value from crc32()? #5

Closed cainus closed 11 years ago

cainus commented 12 years ago

require('crc').crc32('alejandro'); -1429888689

This should be "AAC5A14F" or 2865078607 as far as my testing in other languages goes.

This package appears to get it right too: https://npmjs.org/package/crc32

FYI: I'm on 64-bit node. v 0.8.3

bminer commented 11 years ago

EDIT: This is not a bug. The number returned by crc32 is a Number, not a hexstring. Try doing this:

(require('crc').crc32('alejandro') + Math.pow(2, 32) ).toString(16);
'aac5a14f'

or this...

require('crc').hex32(require('crc').crc32('alejandro') );
'AAC5A14F'

Please close this issue

cainus commented 11 years ago

I wasn't actually talking about the hex string equivalent at all. My problem was basic checksum comparison on a numeric level (using plain ===, without converting to a string in any base).

I didn't realize until I saw your usage here that you were returning a signed int instead of an unsigned one. This is a bit of an unusual return type based on other crc32 libs I've used (eg http://www.wolframalpha.com/input/?i=crc32+%22alejandro%22 ), which makes basic checksum comparison fail, if one of the checksums comes from a different library/language. I'm guessing you did this to get all 32 bits of the javascript's puny "Number"? That makes sense.

Anyway, I'll close it because it works, but you might want to document this. I banged my head on it for a while before switching libraries.

bminer commented 11 years ago

@cainus - Agreed 100%. This should definitely be documented. The other CRC functions (the ones less than 32-bits) return unsigned values. JavaScript's "Number" should be more than capable of handling a 32-bit unsigned value. Maybe this should be changed (perhaps by adding 2^32 to the result)? Maybe re-open this issue so this can be documented???