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

Allowing to parse hex strings into the Integer type #34

Merged
merged 3 commits into from
Jun 10, 2019
Merged

Allowing to parse hex strings into the Integer type #34

merged 3 commits into from
Jun 10, 2019

Conversation

svartalf
Copy link
Contributor

@svartalf svartalf commented Jun 6, 2019

Number representation in the NetBSD proplist implementation proplib is a little bit different from the Apple one, see the PROP_NUMBER(3) page for details.

Implementing all this stuff would vastly decrease the Integer' FromStr implementation performance, so I'm not sure yet if it even should be implemented fully (ex. if representation starts with a 0x then it is an unsigned number), so as a draft, I've added the ability to parse the hexademical strings.

Another option would be to feature-gate it (like #[cfg(target_os = "netbsd")]), but I'm not quite sure about it too — someone might want to parse the NetBSD files in the different OS, right?

Copy link
Owner

@ebarnard ebarnard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to implement this. Not too concerned with integer parsing performance. Could you make a couple of changes though.

src/integer.rs Outdated Show resolved Hide resolved
tests/fuzzer.rs Outdated Show resolved Hide resolved
@svartalf svartalf marked this pull request as ready for review June 7, 2019 11:43
@ebarnard ebarnard merged commit 5ee33d7 into ebarnard:master Jun 10, 2019
@ebarnard
Copy link
Owner

Released in 0.4.2.

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 this pull request may close these issues.

2 participants