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

failure to load highly precise floats #150

Closed
alex-tee opened this issue Mar 12, 2021 · 6 comments
Closed

failure to load highly precise floats #150

alex-tee opened this issue Mar 12, 2021 · 6 comments

Comments

@alex-tee
Copy link

when you have a struct with only a float field inside and you set the value to 1.55331e-40f and attempt to save it, it works. it gets printed as:

---
fval: 1.55331e-40
...

when you try to load it however it rejects it:

# cyaml-MESSAGE: Load: PUSH[0]: start
# cyaml-MESSAGE: Load: Event: STREAM_START
# cyaml-MESSAGE: Load: Handle state start
# cyaml-MESSAGE: Load: PUSH[1]: in stream
# cyaml-MESSAGE: Load: Event: DOC_START
# cyaml-MESSAGE: Load: Handle state in stream
# cyaml-MESSAGE: Load: PUSH[2]: in doc
# cyaml-MESSAGE: Load: Event: MAPPING_START
# cyaml-MESSAGE: Load: Handle state in doc
# cyaml-MESSAGE: Load: Reading value of type 'MAPPING' (pointer)
# cyaml-MESSAGE: Load: Allocation: 0x5555555c1030 (0 + 4 bytes)
# cyaml-MESSAGE: Load: PUSH[3]: in mapping (key)
# cyaml-MESSAGE: Load: Event: SCALAR
# cyaml-MESSAGE: Load: Handle state in mapping (key)
# cyaml-MESSAGE: Load: [fval]
# cyaml-MESSAGE: Load: Event: SCALAR
# cyaml-MESSAGE: Load: Handle state in mapping (value
# cyaml-MESSAGE: Load: Reading value of type 'FLOAT'
# cyaml-MESSAGE: Load:   <1.55331e-40>
# cyaml-MESSAGE: Free: Top level data: 0x5555555c1030
# cyaml-MESSAGE: Free: Freeing key: fval (at offset: 0)
# cyaml-MESSAGE: Free: Freeing: 0x5555555c1030
Bail out! cyaml-FATAL-WARNING: Load: Backtrace:
Bail out! cyaml-FATAL-WARNING:   in mapping field: fval
# cyaml-MESSAGE: Load: POP[3]: in mapping (key)
# cyaml-MESSAGE: Load: POP[2]: in doc
# cyaml-MESSAGE: Load: POP[1]: in stream
# cyaml-MESSAGE: Load: POP[0]: start

cyaml error: Invalid value
@alex-tee alex-tee changed the title failure to load floats failure to load highly precise floats Mar 12, 2021
@alex-tee
Copy link
Author

with latest master I get the same issue, except the error is a bit more user friendly: cyaml-WARNING **: 16:57:44.402: Load: Invalid FLOAT value: 1.55331e-40

@alex-tee
Copy link
Author

alex-tee commented Mar 12, 2021

It seems that those values are outside the bounds allowed by floats, but C allows you to save them in float variables anyway. so this could be seen as a user error - I'm passing bad values. but my program receives some values from other libraries and from the user and I still don't want it to break when trying to load these values back.

so I tried this and it fixes my problem:

diff --git a/src/save.c b/src/save.c
index 52ff1f4..7c47c23 100644
--- a/src/save.c
+++ b/src/save.c
@@ -16,6 +16,7 @@
 #include <stdbool.h>
 #include <assert.h>
 #include <limits.h>
+#include <float.h>
 
 #include <yaml.h>
 
@@ -552,6 +553,15 @@ static const char * cyaml__get_float(
 {
        static char string[64];
 
+  /* clamp */
+  if (value <= FLT_MIN)
+    {
+      value = 1e-37;
+    }
+  else if (value >= FLT_MAX)
+    {
+      value = 1e+37;
+    }
        sprintf(string, "%g", value);
 
        return string;

maybe you can do it in a more tidy way unless you have a better solution.

I think since cyaml allows you to save such a value, it should also not have a problem loading it back. my suggestion is to just log a warning that the value was out of bounds and clamp it like above

EDIT: I guess you could just use strtod() instead of strtof() when loading, that would probably make more sense, and do a (float) cast on it

@alex-tee
Copy link
Author

another idea is to allow user overrides or callbacks before saving so I could clamp the value in my struct (or do any other arbitrary validation): #120

alex-tee added a commit to zrythm/zrythm that referenced this issue Mar 12, 2021
Force use of libcyaml subproject. Use patched version that fixes an
issue with loading out-of-bounds floats. See
tlsa/libcyaml#150 for more info.
@tlsa
Copy link
Owner

tlsa commented Mar 14, 2021

Please test with #151

The old behavior of rejecting floating point values that overflow or underflow can be achieved by setting CYAML_FLAG_STRICT.

@alex-tee
Copy link
Author

just tried it, this fixes the issue. thanks!

@tlsa
Copy link
Owner

tlsa commented Mar 14, 2021

Thanks, merged.

@tlsa tlsa closed this as completed Mar 14, 2021
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