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

Adding type uint64 #520

Closed
JayyCee opened this issue Dec 27, 2019 · 4 comments
Closed

Adding type uint64 #520

JayyCee opened this issue Dec 27, 2019 · 4 comments

Comments

@JayyCee
Copy link

JayyCee commented Dec 27, 2019

I've enhanced the code with adding uint64 functions in my local branch. This uint64 type is needed for my project that needs to handling a full data value of a 64-bit register from a hardware module.

I would like to share my enhancements. What are the steps for me to do to make a pull-request?

--JC

@hawicz
Copy link
Member

hawicz commented Dec 29, 2019

This was asked before by @topilski , see #326
However, the implementation leaves a lot to be desired. In particular, what he suggested adds a whole new "uint" type, but doesn't appear to do anything to deal with overflow of the internal signed storage field, and particular poor behavior when trying to retrieve negative values (i.e. just returns 0).

Also, any attempt to add additional bits is going to increase overhead and have various tradeoffs, and I'm skeptical that it's worth doing that for just a single bit. On the other hand, if you want to bump things up to 128 bits, then things get more interesting. Either way, I'd expect a characterization from you as to what you believe the benefits and drawbacks are/will be, and why it's justified to include the changes you want.

@JayyCee
Copy link
Author

JayyCee commented Dec 31, 2019

I see where you come from regarding adding 64-bit thing.

The single benefit:
Using current master's uint/int type, it gets a wrong value of -1 or the like, when processing an unsigned integer value of 0xFFFFFFFFFFFFFFFF from an hardware register.

After adding uint64_t capable of functions, there is a working function of "json_object_new_uint64(0xFFFFFFFFFFFFFFFF), and then json_obj.to_string() function correctly produces a right number for it.

Trying the 128-bit thing is a very interesting challenge and it is beyond of my capability. Hope you understand.

@dota17
Copy link
Member

dota17 commented Feb 25, 2020

Adding uint64 data type is complicated but not difficult. I have made it in #542 . UINT64 is useful and many json libraries have this feature. I am willing to promote the solution of this issue.
@hawicz @JayyCee How do you think of #542? Need your suggestions. 0.0

@hawicz
Copy link
Member

hawicz commented Mar 10, 2020

Closing this, since #542 has been merged. It adds support for uint64_t w/o adding a new json_type. Overflow should be handled reasonably, by capping at UINT64_MAX, while negative values still end up being returned as 0.

@hawicz hawicz closed this as completed Mar 10, 2020
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

3 participants