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

Fix stat definition for android aarch64 #41752

Closed
wants to merge 1 commit into from

Conversation

malbarbo
Copy link
Contributor

@malbarbo malbarbo commented May 4, 2017

libc::stat for android aarch64 was change in rust-lang/libc#538, this PR update raw::stat to match libc::stat. libc is now tested on aarch64 android, so libc::stat should be correct.

Also changed nlink_t to u64 on x86_64 to match the others android archs (and to satisfy #31551)

@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@malbarbo
Copy link
Contributor Author

malbarbo commented May 4, 2017

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned brson May 4, 2017
@alexcrichton
Copy link
Member

Hm so one reason this whole module is deprecated is that a number of the definitions are known to be incorrect. Is there motivation for correcting them? In theory no code is using these bindings...

@alexcrichton alexcrichton added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 4, 2017
@malbarbo
Copy link
Contributor Author

malbarbo commented May 4, 2017

My motivation is to make it correct... Maybe there are some one using it in the wild. When I read #31551 I thought that std::os::*::raw::*_t could be incorrect, but stat should be correct.

The main problem here is that stat can give wrong results (even segfault if the size is different from libc::stat). The others raw::*_t types do not have these problems.

Is there any problem in making it correct?

@alexcrichton
Copy link
Member

The main thing I'm worried about is breaking code which may inadvertently be working today, unfortunately. My guess is that most code, if using this at all, is broken anyway, though. I just personally lean towards not bothering with these modules as they shouldn't be used anyway and fixing them can require nontrivial effort.

@malbarbo
Copy link
Contributor Author

malbarbo commented May 4, 2017

Ok. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants