Skip to content

Add getsockname shim for connecting and connected sockets#4985

Open
WhySoBad wants to merge 6 commits intorust-lang:masterfrom
WhySoBad:network-socket-getsockname-for-connected-sockets
Open

Add getsockname shim for connecting and connected sockets#4985
WhySoBad wants to merge 6 commits intorust-lang:masterfrom
WhySoBad:network-socket-getsockname-for-connected-sockets

Conversation

@WhySoBad
Copy link
Copy Markdown
Contributor

Hi,

This pull request adds the getsockname shim for connecting and connected sockets. At the moment, we just return 0.0.0.0 in those cases but because connecting sockets are already locally bound we can just return the value from TcpStream::local_addr.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 29, 2026

Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two.
Please remember to not force-push to the PR branch except when you need to rebase due to a conflict or when the reviewer asks you for it.

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Apr 29, 2026
Comment thread src/shims/unix/socket.rs Outdated
Copy link
Copy Markdown
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

Comment thread tests/pass-dep/libc/libc-socket.rs Outdated
Comment on lines +428 to +429
// Because the bound address could be of any local interface, we cannot
// test for localhost.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The socket API would let us bind before calling connect to set the local address, right?
But I guess that's not implemented yet / can't even be implemented with the mio API?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the socket API allows to do this but we cannot represent this with mio because we only have TcpStream::connect there.

Comment thread tests/pass-dep/libc/libc-socket-no-blocking.rs
@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Apr 30, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 30, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@WhySoBad
Copy link
Copy Markdown
Contributor Author

WhySoBad commented May 1, 2026

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels May 1, 2026
Copy link
Copy Markdown
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

Comment thread src/shims/unix/socket.rs Outdated
Comment thread src/shims/unix/socket.rs Outdated
Comment thread src/diagnostics.rs Outdated
Comment thread src/diagnostics.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels May 1, 2026
@rustbot

This comment has been minimized.

@WhySoBad WhySoBad force-pushed the network-socket-getsockname-for-connected-sockets branch from b1ab66a to c748fb7 Compare May 1, 2026 18:55
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 1, 2026

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: Waiting for the PR author to address review comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants