std::os::errno returns platform specific value. fixes #21898 #21932
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (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. 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 CONTRIBUTING.md for more information. |
| @@ -353,7 +353,7 @@ impl IoError { | |||
| /// operating system) between the call(s) for which errors are | |||
| /// being checked and the call of this function. | |||
| pub fn last_error() -> IoError { | |||
| IoError::from_errno(os::errno() as uint, true) | |||
| IoError::from_errno(os::errno() as i32, true) | |||
There was a problem hiding this comment.
I think you may be able to remove this cast now.
|
This may not be precisely where we want to end up, (see my comment), but I think this is "more correct" for now so it's fine by me to merge. |
|
@alexcrichton thank you for the review. I'll take those casts out tonight to finish this off. |
|
⌛ Testing commit e4a7461 with merge 0f4d450... |
|
💔 Test failed - auto-win-32-opt |
|
What's the procedure for these types of platform specific failures? Should I fix the failures in the output and lean on the test suite, or should I go whole hog and set up a windows vm to test before making a patch commit? |
|
Either way is fine, all platforms will need to pass tests for this to land, however. There probably aren't too many problems, though, so setting up a VM may not be necessary. |
|
@alexcrichton it seems @Jormundir has updated, r? |
Changes std::os::errno to return i32, the return type used by the function being delegated to. This is my first contribution, so feel free to give me advice. I'll be happy to correct things.
|
💔 Test failed - auto-win-64-opt |
|
The test failure was due to file permissions? |
|
@bors: retry |
Changes std::os::errno to return i32, the return type used by the function being delegated to. This is my first contribution, so feel free to give me advice. I'll be happy to correct things.
Changes std::os::errno to return i32, the return type used by the function being delegated to.
This is my first contribution, so feel free to give me advice. I'll be happy to correct things.