-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add experimental newlib bindings #646
Conversation
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.
This looks great to me, thanks @FenrirWolf! I don't mind landing this as-is and and the location of this makes sense to me.
src/unix/newlib/mod.rs
Outdated
mod arm; | ||
pub use self::arm::*; | ||
} else { | ||
pub use unsupported_target; |
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.
Mind adding a comment as to the intention here? (a semi-nicer compiler error message)
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.
Basically I've only verified bindings for ARM so it needs to fail in some fashion for things that aren't ARM. the uclibc module does something similar so I just imitated that: https://github.com/rust-lang/libc/blob/master/src/unix/uclibc/mod.rs#L1771-L1782
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.
Oh yeah this is fine, it'd just be good to have a comment 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.
Sure, I can definitely add a comment. Should I also change the unsupported target
module to target_arch_not_implemented
or something similar?
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.
Either's fine by me!
@@ -0,0 +1,566 @@ | |||
use dox::mem; |
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.
I think this may be unused?
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, I forgot to remove that earlier. I'll do that when I push another fix that I need to make.
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.
Or not. Turns out I actually use that import after all. FD_CLR()
and friends depend on it and I'd just forgotten to include them last time around.
@bors: r+ |
📌 Commit 91f6673 has been approved by |
Add experimental newlib bindings I'm not sure how much desire there is for something like this in the libc crate, but I've been working with a newlib-based toolchain for a while and thought I'd throw this PR up and see what happens. These bindings are more specifically targeted towards the devkitARM toolchain from http://devkitpro.org rather than newlib in general. I'd be happy to try making things more platform and toolchain-agnostic, but I'm not completely sure what the best way to do that is. I can move more of the arch-specific bindings to the `arm` folder, but should there also be a `devkitarm` subdirectory to further separate specific bindings from other newlib implementations? There's also the question of if the bindings should live in the `unix` directory in the first place. Newlib aims to provide a unix/posix-like environment and it would be nice to inherit common unix definitions by default, but it can target anything from embedded devices to custom userlands, and as such it often lacks APIs that are common in other libc variants, such as pthreads.
☀️ Test successful - status-appveyor, status-travis |
Bump to 0.2.25 To get newlib bindings (#646) and other stuff on crates.io
Hello @FenrirWolf. I've made #3345, please take a look at it. |
I'm not sure how much desire there is for something like this in the libc crate, but I've been working with a newlib-based toolchain for a while and thought I'd throw this PR up and see what happens.
These bindings are more specifically targeted towards the devkitARM toolchain from http://devkitpro.org rather than newlib in general. I'd be happy to try making things more platform and toolchain-agnostic, but I'm not completely sure what the best way to do that is. I can move more of the arch-specific bindings to the
arm
folder, but should there also be adevkitarm
subdirectory to further separate specific bindings from other newlib implementations?There's also the question of if the bindings should live in the
unix
directory in the first place. Newlib aims to provide a unix/posix-like environment and it would be nice to inherit common unix definitions by default, but it can target anything from embedded devices to custom userlands, and as such it often lacks APIs that are common in other libc variants, such as pthreads.