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

use the MAX constants from <float.h>? #26

Closed
iweindesmedt opened this issue Sep 11, 2016 · 9 comments
Closed

use the MAX constants from <float.h>? #26

iweindesmedt opened this issue Sep 11, 2016 · 9 comments
Labels

Comments

@iweindesmedt
Copy link

compiler warns with
cJSON.c:170:4: warning: floating constant exceeds range of 'double' [-Woverflow]

i think it might be better to use the constants defined in <float.h> throughout the code, like DBL_MAX, etc...

@hustanhuang
Copy link

(maybe not relevant to this issue)

could you please provide your compiler version?
because I can't get the same warning with my -Woverflow option on

*** at *** in ~/t/cJSON> clang -c cJSON.c -Wall -Woverflow
*** at *** in ~/t/cJSON> ls
CMakeLists.txt Makefile       README.md      cJSON.h        cJSON_Utils.c  test.c         tests
LICENSE        README         cJSON.c        cJSON.o        cJSON_Utils.h  test_utils.c
*** at *** in ~/t/cJSON>

my compiler version:

*** at *** in ~/t/cJSON> clang -v
Apple LLVM version 7.3.0 (clang-703.0.31)
Target: x86_64-apple-darwin15.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

@iweindesmedt
Copy link
Author

sure, XC32 (v1.42) http://www.microchip.com/xc32, its for embedded systems

@DaveGamble
Copy link
Owner

Out of interest, what IS your DBL_MAX?

@iweindesmedt
Copy link
Author

printf("%g", DBL_MAX); -> 3.40282e 38
FYI, with LDBL_MAX, it compiles too (can't get it to print though, guess the version of printf doesn't support %Le)

@DaveGamble
Copy link
Owner

DaveGamble commented Oct 9, 2016

Ok, so, you have a non IEEE754-compliant floating point implementation.
What it's complaining about is the 1e60 test which is used to ensure safety when printing huge integers. That number doesn't really pertain to anything in <float.h>, it pertains to ensuring that the number of digits that will be printed doesn't exceed the preallocated buffer size. A proper fix for this involves replacing sprintf with a custom printer, although that feels somewhat overkill.
Perhaps a good fix might be #if (DBL_MAX>1e60) around part of that test...?
Anyone else have any thoughts on this?

@FSMaxB
Copy link
Collaborator

FSMaxB commented Nov 5, 2016

Here it is suggested to use a long double literal for the 1e60, namely 1e60L. @iweindesmedt does that fix the overflow?

@iweindesmedt
Copy link
Author

yes it does, good catch

@FSMaxB FSMaxB closed this as completed in 3ea491c Nov 5, 2016
@DaveGamble
Copy link
Owner

This is not a great change for the master branch. 1e60L is, as you say, a long double, the comparison against which requires a lot of legwork on the FPU (specifically an upcast to long double of the other value). This change worsens performance for JSON print for everyone, whilst fixing a bug for one platform with a non IEEE754 compliant floating point implementation.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Nov 7, 2016

I reverted the commit. This should be easy enough to fix by hand if needed.

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

No branches or pull requests

4 participants