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 ppoll() for all unix platforms #537

Merged
merged 7 commits into from
Feb 24, 2017
Merged

Add ppoll() for all unix platforms #537

merged 7 commits into from
Feb 24, 2017

Conversation

Susurrus
Copy link
Contributor

I'm unsure of whether there is support in OS X for this, and I can't find anything online (so I'm betting there isn't), but I'm going to let this run through CI to confirm.

I'm unsure of whether there is support in OS X for this, and I can't find
anything online (I'm betting there isn't), but I'm going to let this run
through CI to confirm
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@alexcrichton
Copy link
Member

@bors: r+

Thanks!

@bors
Copy link
Contributor

bors commented Feb 23, 2017

📌 Commit cd84475 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Feb 23, 2017

⌛ Testing commit cd84475 with merge 93525ec...

bors added a commit that referenced this pull request Feb 23, 2017
Add ppoll() for all unix platforms

I'm unsure of whether there is support in OS X for this, and I can't find anything online (so I'm betting there isn't), but I'm going to let this run through CI to confirm.
@Susurrus
Copy link
Contributor Author

I have a local fix for the styling, so I already know about that, but I also ran things through my local test suite and found some errors I'm not understanding:

cargo:warning=/home/susurrus/Projects/libc/target/debug/build/libc-test-26b349ee79642880/out/all.c: In function ‘__test_fn_ppoll’:
cargo:warning=/home/susurrus/Projects/libc/target/debug/build/libc-test-26b349ee79642880/out/all.c:2830:24: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
cargo:warning=                 return ppoll;
cargo:warning=                        ^~~~~
cargo:warning=cc1: all warnings being treated as errors

But if I look at that file that function looks correct:

int (*_test_fn_ppoll(void))(struct pollfd*, nfds_t, int, const sigset_t*) {
    return ppoll;
}

I thought maybe it was because the sigset_t struct wasn't getting a struct keyword, but sigwait doesn't seem to have that problem.

@alexcrichton
Copy link
Member

Let's see what CI says, it may just be a const/mut or bit width thing

@Susurrus
Copy link
Contributor Author

That same problem is occurring on Travis, so there's definitely something wrong which I can reproduce locally.

I also pushed a style fix, but it looks like that retriggered CI, so maybe I shouldn't have done that yet...

@Susurrus
Copy link
Contributor Author

Alright, I think I got it. Had the wrong type for timeout.

@Susurrus Susurrus mentioned this pull request Feb 23, 2017
@Susurrus
Copy link
Contributor Author

So turns out this doesn't exist on Mac or NetBSD, or at least not by that name. There are similar different names for pselect() or one of the other functions. But I couldn't find anything by googling, though I did find the name ppollts() for NetBSD, but I don't know if that's right. Any suggestions for how to resolve this one way or another for those platforms?

@alexcrichton
Copy link
Member

It's ok to just leave out this function on those platforms, you can sink the definition farther down the crate hierarchy

@Susurrus
Copy link
Contributor Author

I just pushed it down the stack, but I'm not certain that was correct. OpenBSD and FreeBSD passed their tests, so it's just NetBSD and Mac that don't have it. Should I also expose it in /src/unix/bsd/freebsdlike/mod.rs?

@Susurrus
Copy link
Contributor Author

Alright, looks like I finally figured it out! @alexcrichton Wanna give this a look-see?

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 24, 2017

📌 Commit 88e37f2 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Feb 24, 2017

⌛ Testing commit 88e37f2 with merge b9a0a6a...

bors added a commit that referenced this pull request Feb 24, 2017
Add ppoll() for all unix platforms

I'm unsure of whether there is support in OS X for this, and I can't find anything online (so I'm betting there isn't), but I'm going to let this run through CI to confirm.
@bors
Copy link
Contributor

bors commented Feb 24, 2017

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

@bors bors merged commit 88e37f2 into rust-lang:master Feb 24, 2017
homu added a commit to nix-rust/nix that referenced this pull request Feb 25, 2017
Add ppoll()

This will currently fail CI, as the necessary changes haven't hit libc yet. That is tracked in rust-lang/libc#537. This does build on my computer using the changes tracked on that PR, so I'd appreciate any visual review of this code as it should be "done".

I also wanted to get this submitted so hopefully it'd be in the queue for the 0.8 release.
homu added a commit to nix-rust/nix that referenced this pull request Feb 25, 2017
Add ppoll()

This will currently fail CI, as the necessary changes haven't hit libc yet. That is tracked in rust-lang/libc#537. This does build on my computer using the changes tracked on that PR, so I'd appreciate any visual review of this code as it should be "done".

I also wanted to get this submitted so hopefully it'd be in the queue for the 0.8 release.
homu added a commit to nix-rust/nix that referenced this pull request Feb 26, 2017
Add ppoll()

This will currently fail CI, as the necessary changes haven't hit libc yet. That is tracked in rust-lang/libc#537. This does build on my computer using the changes tracked on that PR, so I'd appreciate any visual review of this code as it should be "done".

I also wanted to get this submitted so hopefully it'd be in the queue for the 0.8 release.
danielverkamp pushed a commit to danielverkamp/libc that referenced this pull request Apr 28, 2020
- `usad8`: Sum of 8-bit absolute differences
- `usad8a`: Sum of 8-bit absolute differences and constant (usad8(a, b) + c)
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