Skip to content
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 i8 (64-bit integer) parameters #21

Closed
baalexander opened this issue Aug 3, 2011 · 6 comments
Closed

Support i8 (64-bit integer) parameters #21

baalexander opened this issue Aug 3, 2011 · 6 comments

Comments

@baalexander
Copy link
Owner

Originally reported by ProjectMoon in Issue #16:

This returns a very small response, but the callback gives me null as an answer.

get_upload_rate returns me something like this:

<?xml version="1.0" encoding="UTF-8"?>
<methodResponse>
  <params>
    <param><value><i8>296960</i8></value></param>
  </params>
</methodResponse>
@blaenk
Copy link
Contributor

blaenk commented Aug 3, 2011

Maybe node-int64 can help?

@baalexander
Copy link
Owner Author

@blaenk - That int 64 module looks like the most straightforward approach. I'll add that with the next set of updates.

@blaenk
Copy link
Contributor

blaenk commented Oct 21, 2011

@baalexander: I need i8 datatype support for a project. That is, not necessarily 64-bit support, just that the client I'm interfacing with returns all integer responses as i8 datatypes. The above response you mentioned returns null/undefined because the xmlrpc-parser does not parse i8 datatypes from the response. I've added support for i8 datatypes (literally just two simple additions) but I wanted to ask you what way you thought would be best to handle it.

There are two issues. One is the one I already mentioned, of grabbing the actual i8 values out of the response. The second is parsing them into a javascript value using parseInt. Since I've read node.js has no support for 64-bit integers. I tried parseInt on a big number in the REPL and it seemed to have worked, but it seems like the maximum value it accurately parses is Math.pow(2, 53).

I've also read it's possible to just read it as a float (parseFloat), which I also confirm to have worked. I have a feeling this is a better solution but I can't say for sure.

Now, the module I previously mentioned, node-int64, alleges to handle this, but the module can only initialize an Int64 using a hexadecimal string, a Number class, or two 32-bit hi/lo values. The problem is that in the above case, and I imagine per XMLRPC spec, the i8 values are passed already as the actual big integers, so I can't think of a way of passing that into the Int64 class.

In my opinion, I think we'd be fine by simply using parseFloat as I have read elsewhere. After all, this is an inherent limitation of node.js by way of V8, so the module can't be held to blame. In fact, I've seen other modules which disclose this limitation.

Let me know what you think, so I may file a pull request.

@blaenk
Copy link
Contributor

blaenk commented Oct 21, 2011

Here is some more information from the V8 mailing list. It appears using parseFloat is the correct course of action.

If you prefer, however, we can use bigdecimal.js. I've tested this and it works too, but I think it might be more predictable to the user if we use parseFloat, given node.js' already well-known limitation and it seems this is the way we work around this for now.

@baalexander
Copy link
Owner Author

@blaenk - My main reluctance with node-int64 (or bigdecimal) was introducing a whole other datatype into the module. My guess is most of the times, the full precision of i8 is not required and a double level precision may be good enough. I'd happily accept a pull request for using parseFloat for i8 types.

If there's enough demand for full precision support for 64-bit integers, we can always introduce that later.

@blaenk
Copy link
Contributor

blaenk commented Oct 21, 2011

@baalexander: Great. I'll get that pull request up then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants