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

#![no_std] support #45

Closed
whitequark opened this issue Jan 8, 2018 · 5 comments
Closed

#![no_std] support #45

whitequark opened this issue Jan 8, 2018 · 5 comments

Comments

@whitequark
Copy link

whitequark commented Jan 8, 2018

Looks like the only hard std dependency is std::io::Cursor here, which really should use goblin::peek_bytes but that is by some mistake covered under if_std!. oh wow everything transitively depends on std::io my eyes my eyes are burning

@m4b
Copy link
Contributor

m4b commented Jan 18, 2018

The goblin parser can be made no std in two ways:

  1. Can write a new allocationless parser, and which also probably doesn’t worry about endianness
  2. Can update current parser to not allocate. The lazy transducer PR is an example of how to do this with minimal changes. The refactor will be fairly straightforward and clients probably won’t have to change their code. (Only need to add no_std feature flag and make rayon options to lazy transducer which would take about 15 minutes)

I think 2 would be cooler, and less maintenance burden, but it would also remove niceties of preallocated headers, etc. I guess it’s not too important tho?

  1. Would probably be easier, maybe.

In either case have to decide if the automatic struct readers custom derives from scroll are moved into no_std.

I had trouble in the past getting custom derives working on no std but I probably just did something stupid ?

A downside of this is compile times for no std, since it’ll pull in syn, etc.

A downside of not doing it is writing header readers by hand for all the different binary formats again, which will suck and will have to be tested (and fuzzed) all over again. I’m also not sure I want to take on that maintenance burden.

Anyway, if this is of interest, someone can open an issue on goblin repo and flesh out there.

@philipc
Copy link
Contributor

philipc commented Jan 25, 2018

I had trouble in the past getting custom derives working on no std

I think the problem is the derives use ::std in the generated code, and so extern crate core as std; in goblin is enough to get these working (or something similar exported in scroll).

@whitequark Are you still working on this? If not, I might be able to do some work on it.

@whitequark
Copy link
Author

I am not.

@m4b
Copy link
Contributor

m4b commented Jan 25, 2018

I actually realized the feature flags won’t have to modified greatly or cause larger compile times when using less stuff. If the parser is made no std, and people just want the struct defs, will be the same flags as before, and same compile time. If they want parser will be same, just happens to be no std

@m4b
Copy link
Contributor

m4b commented Jan 25, 2018

@philipc yea you’re totally right about std uses. I think I tried pulling out all the std uses and make them a constant value in the generated code but I couldn’t get that working either. Anyway should be easy fix on scroll_derive side.

Only problem is it forces anyone using it to have no_std and/or the extern core :/

Maybe a std feature flag for the derive would help ...?

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

3 participants