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
Call out to the memchr crate for memchr and memrchr #10
Conversation
c-scape/Cargo.toml
Outdated
@@ -18,6 +18,7 @@ rsix = "0.22.3" | |||
memoffset = "0.6" | |||
realpath-ext = "0.1.0" | |||
compiler_builtins = "0.1.50" | |||
memchr = { version = "2.4.1", default-features = false } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, c_scape
doesn't need to be no_std
; it just needs to make sure it supports any libc
functions that the parts of std
it uses use :-). So unless there are other considerations, I suggest removing the default-features = false
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, didn't think that through clearly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't there the risk of a cyclic dependency between say extern "C" fn memchr
and the cpu feature detection code in libstd?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, there is :-). There are risks of cyclic dependencies between core
/alloc
and c-scape
as well; it's the nature of the space c-scape
is operating in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To say a little more here: I guess core
doesn't call libc, but AIUI alloc
does (by default).
And the other side here is that no_std
is really limiting; we don't get useful things like OsStr
or CStr
or Ipv4Addr
or other things, and we have fewer options in terms of third-party crates we can use. And ultimately, the point of mustang
is to run on Linux, and sometimes the hoops that no_std
makes us jump through aren't meaningful, because we do have an OS. So it comes down to a kind of somewhat-educated guess, that using std
will work out well enough to be worth the risks and downsides.
Looks good! And good catch on the (weird) C signature :-). |
See #5.
Note: I've changed the signatures of
memchr
andmemrchr
to match the (weird) C signatures, which take a const pointer but return a mut pointer (e.g.).