Skip to content

Conversation

joboet
Copy link
Member

@joboet joboet commented Sep 30, 2025

This PR contains three improvements to the socket-based networking implementation (aa1263e is just to add the now missing unsafe). Best reviewed commit-by-commit.

@rustbot rustbot added O-unix Operating system: Unix-like O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 30, 2025

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@joboet
Copy link
Member Author

joboet commented Sep 30, 2025

r? libs
You've been assigned so many of my PRs already...

@rustbot rustbot assigned tgross35 and unassigned ibraheemdev Sep 30, 2025
let mut len = size_of_val(&storage) as c::socklen_t;
let mut storage = MaybeUninit::<c::sockaddr_storage>::uninit();
let mut len = size_of::<c::sockaddr_storage>() as c::socklen_t;
let sock = self.inner.accept(storage.as_mut_ptr() as *mut _, &mut len)?;
Copy link
Member

Choose a reason for hiding this comment

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

perhaps add let storage = storage.assume_init() here?

}

pub fn new_raw(fam: i32, ty: i32) -> io::Result<Socket> {
pub fn new(fam: i32, ty: i32) -> io::Result<Socket> {
Copy link
Member

Choose a reason for hiding this comment

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

The other impls seem to use c_int instead of i32 for these parameters.

Copy link
Member Author

Choose a reason for hiding this comment

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

hermit_abi uses i32 for socket, so it's probably best to use it here as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-unix Operating system: Unix-like O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants