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

Replace the Sockaddr enum with a union #1684

Merged
merged 1 commit into from Mar 22, 2022
Merged

Conversation

asomers
Copy link
Member

@asomers asomers commented Mar 20, 2022

The SockAddr enum is quite large, and the user must allocate space for
the whole thing even though he usually knows what type he needs.
Furthermore, thanks to the sa_family field, the sockaddr types are
basically an enum even in C.

So replace the ungainly enum with a SockaddrLike trait implemented by
all sockaddr types and a SockaddrStorage union that has safe accessors.

Also, deprecate InetAddr, which only existed to support SockAddr.

Supplants #1504
Fixes #1544

@asomers asomers marked this pull request as ready for review March 20, 2022 18:19
@asomers asomers requested a review from rtzoeller March 20, 2022 18:19
Copy link
Collaborator

@rtzoeller rtzoeller left a comment

Choose a reason for hiding this comment

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

LGTM overall.

I ran the tests locally on DragonFlyBSD as well with no issues.

test/sys/test_socket.rs Show resolved Hide resolved
src/sys/socket/mod.rs Show resolved Hide resolved
src/sys/socket/addr.rs Show resolved Hide resolved
src/sys/socket/addr.rs Show resolved Hide resolved
src/sys/socket/addr.rs Outdated Show resolved Hide resolved
@rtzoeller
Copy link
Collaborator

@asomers I think this just needs to be squashed; I'm happy with the changes.

The SockAddr enum is quite large, and the user must allocate space for
the whole thing even though he usually knows what type he needs.
Furthermore, thanks to the sa_family field, the sockaddr types are
basically an enum even in C.

So replace the ungainly enum with a SockaddrLike trait implemented by
all sockaddr types and  a SockaddrStorage union that has safe accessors.

Also, deprecate InetAddr, which only existed to support SockAddr.

Supplants nix-rust#1504
Fixes nix-rust#1544
@asomers
Copy link
Member Author

asomers commented Mar 22, 2022

bors r=rtzoeller

@bors bors bot merged commit 0fe6682 into nix-rust:master Mar 22, 2022
@asomers asomers deleted the sockaddr_union3 branch March 22, 2022 03:40
asomers added a commit to asomers/nix that referenced this pull request May 31, 2022
asomers added a commit to asomers/nix that referenced this pull request May 31, 2022
bors bot added a commit that referenced this pull request May 31, 2022
1729: Enable SockaddrStorage::{as_link_addr, as_link_addr_mut} on Linux. r=rtzoeller a=asomers

This was an oversight from #1684.

Fixes #1728

Co-authored-by: Alan Somers <asomers@gmail.com>
rtzoeller pushed a commit to rtzoeller/nix that referenced this pull request Jul 17, 2022
@SteveLauC SteveLauC mentioned this pull request Oct 4, 2023
3 tasks
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.

Change Sockaddr to a union
2 participants