-
Notifications
You must be signed in to change notification settings - Fork 423
Refactor socketpair tests to use utility functions for reading/writing #4770
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
base: master
Are you sure you want to change the base?
Refactor socketpair tests to use utility functions for reading/writing #4770
Conversation
|
Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two. |
|
@hulxv please do not submit half a dozen PRs at once. That feels like a DoS attack. |
oh sorry for that xD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar comments to the other PR. :)
That's why it's better to wait after submitting the first PR in a series -- the same comments are likely to apply many times. I will not look at your remaining PRs until we finished these first two, to avoid having to repeat the same comments.
|
|
||
| #[path = "../../utils/libc.rs"] | ||
| mod libc_utils; | ||
| use libc_utils::{errno_check, errno_result, read_all_into_array, write_all_from_slice}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| use libc_utils::{errno_check, errno_result, read_all_into_array, write_all_from_slice}; | |
| use libc_utils::*; |
| let data = b"abc"; | ||
| write_all_from_slice(fds[0], data).unwrap(); | ||
| let mut buf2: [u8; 5] = [0; 5]; | ||
| let res = unsafe { libc::read(fds[1], buf2.as_mut_ptr().cast(), buf2.len() as libc::size_t) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to #4768, this should use helpers instead of directly invoking read.
| let data = b"123"; | ||
| write_all_from_slice(fds[1], data).unwrap(); | ||
| let mut buf4: [u8; 5] = [0; 5]; | ||
| let res = unsafe { libc::read(fds[0], buf4.as_mut_ptr().cast(), buf4.len() as libc::size_t) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another place that should use a helper.
| // Reading the other end should return that data, then EOF. | ||
| let mut buf: [u8; 5] = [0; 5]; | ||
| let res = | ||
| unsafe { libc_utils::read_all(fds[1], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another helper candidate.
| assert_eq!(&buf[0..3], "123".as_bytes()); | ||
| assert_eq!(&buf[0..3], b"123"); | ||
| let res = | ||
| unsafe { libc_utils::read_all(fds[1], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And another one
| let data = "12345".as_bytes().as_ptr(); | ||
| let res = unsafe { libc_utils::write_all(fds[1], data as *const libc::c_void, 5) }; | ||
| assert_eq!(res, 5); | ||
| let data = b"12345"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable seems to be used only once, please inline it. (Same everywhere else in this PR, and in your other PRs as well.)
|
Reminder, once the PR becomes ready for a review, use |
Related to #4725