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

libnative/io/net, file_unix: fix offset parameters. #18658

Closed
wants to merge 1 commit into from

Conversation

nodakai
Copy link
Contributor

@nodakai nodakai commented Nov 5, 2014

Fixes #17791

Also introduce new hellper functions check_error() and
check_error_unit().

Signed-off-by: NODA, Kai <[email protected]>
@rust-highfive
Copy link
Collaborator

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@alexcrichton
Copy link
Member

I think for now this may want to wait until #18557 is merged, and in the meantime is there any way that we can test this functionality? This all seems incredibly subtle and very easy to mess up, and it would be very nice to have regression tests making sure that we're not going too far into the weeds here.

I'm also a little uncomfortable with how these functions are turning out generically, there's a growing number of functions in libnative to deal with errors (check_error, check_error_unit, keep_going, mkerr_libc, ...). Isn't check_error_unit the same as mkerr_libc, and isn't the nonzero case of check_error the same as mkerr_libc?

@nodakai
Copy link
Contributor Author

nodakai commented Nov 8, 2014

@alexcrichton I wasn't aware The Great I/O Reorganization was in progress when I submitted this patch. I'll wait for its completion and will work on regression tests meantime.

I'll also address the issue of prolification of error check utilites.

@alexcrichton
Copy link
Member

Aha! It has now landed. Feel free to ping me when this is rebased and I'll take another look. Thanks @nodakai!

@steveklabnik
Copy link
Member

@nodakai what's up with this PR?

@nodakai
Copy link
Contributor Author

nodakai commented Dec 2, 2014

@steveklabnik I've never forget this but checking consistency of error code handling for tons of system calls takes me long...

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to reopen with a rebase!

@nodakai nodakai deleted the libnative-fix-pwrite branch March 26, 2016 07:20
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.

libnative/io: pwrite() is broken.
4 participants