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.

Sign up for GitHub

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

Jump to bottom

Fix android SA_* constants #511

Merged
merged 1 commit into from
May 25, 2019
Merged

Fix android SA_* constants #511

merged 1 commit into from
May 25, 2019

Conversation

malbarbo
Copy link
Contributor

@malbarbo malbarbo commented Feb 3, 2017

Trying to crosscompile wait-timeout to armv7-linux-androideabi I found that SA_* had different types in libc, but they have to be the same because they are used in bitwise operations.

@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

Thanks for the PR! Unfortunately this'll be a breaking change to libc so we'll have to hold off on this for now, but otherwise we can be sure to merge on the next bump!

@malbarbo malbarbo mentioned this pull request Feb 10, 2017
Closed
@kamalmarhubi
Copy link
Contributor

@alexcrichton if you have time, could you add a label for all these fixes-that-are-breaking PRs? :-)

@alexcrichton
Copy link
Member

Sure yeah

@malbarbo
Copy link
Contributor Author

malbarbo commented Feb 25, 2017 •
edited
Loading

@alexcrichton It this really a breaking change? Can we consider it a bug fix instead? Some others PR with type changes were accepted, for example #489, #516, #523, #533.

@alexcrichton
Copy link
Member

@malbarbo I think #489 didn't have any deletions, so was that a mistake? All the others I believe the breakage was contained to Solaris, a tier 3 platform. Android, however, is tier 2 :(

@brinsche brinsche mentioned this pull request Feb 27, 2017
Closed
@malbarbo
Copy link
Contributor Author

@alexcrichton #489 was my bad. Thank you for the clarifications.

@bors
Copy link
Contributor

bors commented Mar 1, 2017

☔ The latest upstream changes (presumably #538) made this pull request unmergeable. Please resolve the merge conflicts.

@roblabla
Copy link
Contributor

roblabla commented Jun 27, 2017 •
edited
Loading

So I just looked at sigaction from the android-24 platform in the NDK, and it seems to me that this PR is wrong. In the NDK, both arch-arm and arch-arm64 have sa_flag from sigaction set to unsigned long. You can see for yourself :

So, as far as I'm aware, this should really be defined as c_ulong directly under android/. Am I missing something ?

EDIT: Nevermind me. sa_flag is defined as c_uint on 64-bit platforms there https://android.googlesource.com/toolchain/prebuilts/ndk/r13/+/master/platforms/android-24/arch-arm/usr/include/signal.h#84

So sa_flag is c_uint on 64-bit and c_ulong on 32-bit.

@roblabla roblabla mentioned this pull request Jun 28, 2017
Merged
roblabla added a commit to roblabla/nix that referenced this pull request Jul 4, 2017
@roblabla
Allow casting a flag to a type in libc_macros!
78e3455 
This is necessary because certain flags in libc have different types, generally
due to a mistake when adding the flags to the libc. See
rust-lang/libc#511 for an example of such a
discrepency.
roblabla added a commit to roblabla/nix that referenced this pull request Jul 4, 2017
@roblabla
Allow casting a flag to a type in libc_bitflags!
7c02039 
This is necessary because certain flags in libc have different types, generally
due to a mistake when adding the flags to the libc. See
rust-lang/libc#511 for an example of such a
discrepency.
roblabla added a commit to roblabla/nix that referenced this pull request Jul 4, 2017
@roblabla
Allow casting a flag to a type in libc_bitflags!
26a0a48 
This is necessary because certain flags in libc have different types, generally
due to a mistake when adding the flags to the libc. See
rust-lang/libc#511 for an example of such a
discrepency.
Susurrus pushed a commit to Susurrus/nix that referenced this pull request Jul 4, 2017
@roblabla @Susurrus
Allow casting a flag to a type in libc_bitflags!
b422927 
This is necessary because certain flags in libc have different types, generally
due to a mistake when adding the flags to the libc. See
rust-lang/libc#511 for an example of such a
discrepency.
@gnzlbg
Copy link
Contributor

gnzlbg commented Feb 20, 2019

@malbarbo can you rebase this ?

We can give this a try to see if anybody is affected by these changes, and if they aren't then just do them.

@malbarbo
Copy link
Contributor Author

Done.

@gnzlbg
Copy link
Contributor

gnzlbg commented May 25, 2019

@bors: r+

@bors
Copy link
Contributor

bors commented May 25, 2019

📌 Commit e2bbeed has been approved by gnzlbg

gnzlbg added a commit to gnzlbg/libc that referenced this pull request May 25, 2019
@gnzlbg
Rollup merge of rust-lang#511 - malbarbo:android_sa, r=gnzlbg
c6202b2 
Fix android SA_* constants

Trying to crosscompile wait-timeout to armv7-linux-androideabi I found that SA_* had different types in libc, but they have to be the same because they are used in bitwise operations.
@gnzlbg gnzlbg mentioned this pull request May 25, 2019
Closed
bors added a commit that referenced this pull request May 25, 2019
@bors
Auto merge of #1368 - gnzlbg:rollup-ytrx9bp, r=gnzlbg
8adea96 
Rollup of 4 pull requests

Successful merges:

 - #503 (Set the type for S_ISUID, S_ISGID and S_ISVTX correctly)
 - #511 (Fix android SA_* constants)
 - #1358 (Add powerpc64-unknown-freebsd target)
 - #1364 (Deprecate mach APIs: users should use the `mach` crate instead.)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request May 25, 2019
@bors
Auto merge of #511 - malbarbo:android_sa, r=gnzlbg
0f395c6 
Fix android SA_* constants

Trying to crosscompile wait-timeout to armv7-linux-androideabi I found that SA_* had different types in libc, but they have to be the same because they are used in bitwise operations.
@bors
Copy link
Contributor

bors commented May 25, 2019

⌛ Testing commit e2bbeed with merge 0f395c6...

@bors
Copy link
Contributor

bors commented May 25, 2019

☀️ Test successful - checks-cirrus, checks-travis, status-appveyor
Approved by: gnzlbg
Pushing 0f395c6 to master...

@bors bors merged commit e2bbeed into rust-lang:master May 25, 2019
danielverkamp pushed a commit to danielverkamp/libc that referenced this pull request Apr 28, 2020
@gnzlbg
Fix PartialEq issue rust-lang#511
4751cb9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
breakage-candidate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@malbarbo @rust-highfive @alexcrichton @kamalmarhubi @bors @roblabla @gnzlbg