-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Issue with 64-bit integers on 86_x64 Linux #509
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
Comments
Seems reasonable. |
Better use |
I was thinking the int64_t or uint64_t would be a good route to go. I haven't had a chance to seriously look into fixing this yet, but I have made a fork and will create a pull-request once I have something that works. Thanks for looking into the issue. |
Please let me know if you spot a problem, so we can revert this quickly. |
Thanks for fixing this issue. This seems to fix the issue we were having in our CentOS 7 build. However, I was playing around with this myself and I believe it might be a slightly better option to include the |
I'm afraid that might depend on the specific version of VisualStudio. Actually, we've tended to drop support for older compilers with the 1.y.z branch, but I still hesitate to change such a thing since I hate to break anyone. |
This broke binary-compatibility! I'm discussing this with O/S vendors now. Since the damage is done, and since the old version was actually wrong, we might keep it. Or we might have to revert this. Not sure yet... |
We might also need to |
Hello, guys! I also faced this problem a long time ago. I think you should not worry about using these typedefs in VS version also, as they are part of C++ standard now. cdunn2001, you mentioned broken binary-compatibility. What you are talking about? I'm using this fixed version more than a year. But I really want this fix became a part of jsoncpp library. Also, it is totally crazy to install/unpack jsoncpp on linux to /usr/include/?jsoncpp?/json as all samples are referring to just including <json/json.h>. I found a lot of questions from less experienced programmers and the best they achieved is including like <jsoncpp/json/json.h>. Thanks! Hope, together we can make it better! :) |
Well, this:
If the sizes change, then the headers are not binary-compatible with existing builds. I'm not sure how to deal with that. Since we're versioning now, maybe it's ok. But I'm not sure. There are faster and more C++-14-compliant JSON libraries. The main reasons to use JsonCpp are flexibility and back-compatibility. I'm not opposed to any particular change though. Submit a pull-request and see who wants to discuss it. |
Fixed by change to int64_t and uint64_t described by @sedapsfognik above. |
There is a problem trying to use the 64-bit integer options on x86_64 Linux versions as the json/config.h file defines Int64 as "long long int". However, x86_64 Linux uses only "long int" to define 64-bit integers. An option should probably be added to the define section when it detects a Linux system to be x86_64. The Int64 typedef should be set as "long int" instead. The UInt64 should follow the same pattern.
The text was updated successfully, but these errors were encountered: