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

Please clarify that floats are not supported and do not attempt to parse them #16

Open
tliron opened this issue Dec 1, 2024 · 8 comments · May be fixed by #18
Open

Please clarify that floats are not supported and do not attempt to parse them #16

tliron opened this issue Dec 1, 2024 · 8 comments · May be fixed by #18

Comments

@tliron
Copy link

tliron commented Dec 1, 2024

Right now, you would only find this out in the Crates documentation. And, honestly, it is a fairly tiny note for what is ostensibly a major missing feature.

Otherwise, the library does attempt to parse floats, except we get wrong data. It seems that the developer started to work on an implementation but didn't finish it. I think it would be better to return an error rather than wrong data.

@NicolasDP
Copy link
Contributor

You are right. Floating points are not supported (yet?) and this is not well enough documented.

@tliron
Copy link
Author

tliron commented Dec 2, 2024

For what it's worth, I switched to Borc. Perhaps you can borrow some solutions from Borc? Hopefully they can work for no-std. (There are a million CBOR libraries for Rust, but almost none that are event-based and comprehensive.)

Thank you for your library!

@NicolasDP
Copy link
Contributor

Thanks for letting me know. Borc has an interesting approach and it looks like they took seriously to be close to the RFC. I do not think they are no_std though. But please let me know if this is the case. I know @SimonIT is interested about a no_std CBOR library too.

cbor_event is an old project I keep wanting to come back to in order to address some API issues (not enough event based for example). Also, I'd like to remove the Serialise and Deserialise traits (maybe put it in a different crate?) as they don't really make sense in the context of event based API. Hopefully I should have some time to work on that in a few weeks and also remove the dependency on std.

@SimonIT
Copy link
Contributor

SimonIT commented Dec 2, 2024

I'm not sure what you mean by getting wrong data when parsing floats. We used f32 successfully in a project with the current implementation (tho with cddl-codegen and my open PR there). f64 should also work, there are tests for it:

cbor_event/src/de.rs

Lines 957 to 975 in 2b6330a

#[test]
fn float64() {
let vec = vec![0xfb, 0x3f, 0xf1, 0x99, 0x99, 0x99, 0x99, 0x99, 0x9a];
let mut raw = Deserializer::from(vec);
let float = raw.float().unwrap();
assert_eq!(float, 1.1);
}
#[test]
fn float32() {
let vec = vec![0xfa, 0x47, 0xc3, 0x50, 0x00];
let mut raw = Deserializer::from(vec);
let float = raw.float().unwrap();
assert_eq!(float, 100000.0);
}

cbor_event/src/se.rs

Lines 911 to 933 in 2b6330a

#[test]
fn special_float() {
assert!(test_special(
Special::Float(1.1),
[0xfb, 0x3f, 0xf1, 0x99, 0x99, 0x99, 0x99, 0x99, 0x9a].as_ref()
));
assert!(test_special(
Special::Float(-4.1),
[0xfb, 0xc0, 0x10, 0x66, 0x66, 0x66, 0x66, 0x66, 0x66].as_ref()
));
assert!(test_special(
Special::Float(f64::INFINITY),
[0xfb, 0x7f, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00].as_ref()
));
assert!(test_special(
Special::Float(f64::NAN),
[0xfb, 0x7f, 0xf8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00].as_ref()
));
assert!(test_special(
Special::Float(f64::NEG_INFINITY),
[0xfb, 0xff, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00].as_ref()
));
}

We also used minicbor as an alternative for true no_std support, because #15 was kinda hard to make

@tliron
Copy link
Author

tliron commented Dec 2, 2024

I'm not sure what you mean by getting wrong data when parsing floats.

I mean that I used cbor_event to parse in some data and the floats all had wrong values in Special::Float. Integers and other values were fine. When that happened and I tried to look into it, I discovered the note in the documentation that specials are supported "except floating points".

@NicolasDP
Copy link
Contributor

My bad, it was done here #10

@tliron
Copy link
Author

tliron commented Dec 2, 2024

Yay! I have moved to another library (borc) as I don't have a need for no_std, but I'm sure this library will be very valuable to some users.

@SimonIT
Copy link
Contributor

SimonIT commented Dec 2, 2024

My bad, it was done here #10

Yes, that's why I'm a bit confused, where did the wrong values come from? Maybe you were using f16? This is not supported as rust had no f16 type, but as far as I can see at the moment they are planning something like this: rust-lang/rust#116909

@SimonIT SimonIT linked a pull request Dec 7, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

3 participants