search

chrisdew / protobuf

Protocol Buffers for Node.JS
http://code.google.com/p/protobuf-for-node/
Apache License 2.0
234 stars 71 forks source link

Represent (u)int64 fields with Buffers #16

Closed seishun closed 11 years ago

seishun commented 12 years ago

This library currently tries to represent 64-bit signed and unsigned integers as Numbers, which leads to precision problems. I've changed the behavior to parse them into Buffers, and accept Buffers when serializing. The serialization code matches the old behavior when not passed a Buffer.

chrisdew commented 11 years ago

This pull request breaks the existing functionality of turning Int64s into JavaScript Numbers. I agree that the conversion can lose data, if the numbers are bigger than 1<<56 iirc.

For this, and #7, we need to add some parameterisation (at import time?) of conversions.

seishun commented 11 years ago

It's extremely unlikely anyone is using this module to parse (u)int64s, as they would quickly realize it's unreliable and either give up on it or submit an issue. There's no point in preserving a broken functionality.

If someone really wants a Number, then can always do:

buf.readInt32LE(0) + (buf.readInt32LE(4) << 32)

chrisdew commented 11 years ago

No, we need a general solution for 'formatting'. I use int64s as numbers (with bounds checking). They never even exceed 2^32, but they're encoded as int64s in the PB format :-(

trevex commented 11 years ago

An interesting approach is used for longs here (originally from google closure lib). They basically splited the long in two 32bit representations... Unfortunately I currently don't have the time to dig deeper in protobuf and the implementation right now...

seishun commented 11 years ago

I've thought about this approach, however, creating and accessing an arbitrary JS object from C++ is quite tricky. Besides, one could easily get the low and high parts by using readInt32LE(0) and readInt32LE(4).

chrisdew commented 11 years ago

obsoleted by #19