Skip to content

Add UDS/Pipe to writev()/Fix UDS EPIPE on send #250

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

Merged
merged 9 commits into from
May 25, 2024
Merged

Conversation

rennergade
Copy link
Contributor

Description

Fixes # (issue)

  • implements pipes and unix sockets for send()
  • adds sending EPIPE for unix domain sockets on send/writev

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Added unix domain writev test
Lamp tests
Test suite

Checklist:

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

for _iov in 0..iovcnt {
unsafe {
let iovec = *ptr;
assert!(!ptr.is_null());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is performed after dereferencing the pointer, which is not right to me. This assertion should be placed before dereferencing to prevent undefined behavior.

@@ -514,7 +514,9 @@ mod tests {
let emulated_file = EmulatedFile::new(file_path.clone(), file_content.len()).unwrap();

let mut buffer = vec![0; file_content.len()];
let bytes_read = emulated_file.readat(buffer.as_mut_ptr(), buffer.len(), 0).unwrap();
let bytes_read = emulated_file
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

everything in this file is just rust fmt fixing what I didn't format in the read_at PR

Copy link

@yzhang71 yzhang71 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes LGTM!

Copy link
Member

@Yaxuan-w Yaxuan-w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall logic makes sense to me

@rennergade rennergade merged commit 20a5d75 into develop May 25, 2024
1 of 2 checks passed
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.

3 participants