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

Add experimental newlib bindings #646

Merged
merged 1 commit into from
Jul 6, 2017
Merged

Conversation

FenrirWolf
Copy link
Contributor

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.

Copy link
Member

@alexcrichton alexcrichton left a 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.

mod arm;
pub use self::arm::*;
} else {
pub use unsupported_target;
Copy link
Member

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)

Copy link
Contributor Author

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

Copy link
Member

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

Copy link
Contributor Author

@FenrirWolf FenrirWolf Jul 6, 2017

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?

Copy link
Member

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;
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jul 6, 2017

📌 Commit 91f6673 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jul 6, 2017

⌛ Testing commit 91f6673 with merge 375d773...

bors added a commit that referenced this pull request Jul 6, 2017
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.
@bors
Copy link
Contributor

bors commented Jul 6, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 375d773 to master...

@bors bors merged commit 91f6673 into rust-lang:master Jul 6, 2017
@FenrirWolf FenrirWolf deleted the newlib branch July 6, 2017 19:37
@FenrirWolf FenrirWolf mentioned this pull request Jul 7, 2017
bors added a commit that referenced this pull request Jul 7, 2017
Bump to 0.2.25

To get newlib bindings (#646) and other stuff on crates.io
@FenrirWolf FenrirWolf restored the newlib branch July 9, 2017 21:39
@FenrirWolf FenrirWolf deleted the newlib branch July 18, 2017 03:10
@zetanumbers zetanumbers mentioned this pull request Sep 8, 2023
@zetanumbers
Copy link

zetanumbers commented Sep 8, 2023

Hello @FenrirWolf. I've made #3345, please take a look at it. I would like to know where did you get these values from, so i could try to not break it with this PR. As i've said, i've looked at devkitPro/newlib and i haven't found anything like your values there. I think you got these values from devkitPro/libctru and i'm not sure if #3345 would or not break anything. Please help me figure that out.

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.

4 participants