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.

Sign up for GitHub

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

Jump to bottom

recvmsg should take &mut [u8] instead of &mut Vec<u8> for cmsg_buffer #2523

Closed
erenon opened this issue Oct 21, 2024 · 2 comments · Fixed by #2524
Closed

recvmsg should take &mut [u8] instead of &mut Vec<u8> for cmsg_buffer #2523

erenon opened this issue Oct 21, 2024 · 2 comments · Fixed by #2524

Comments

@erenon
Copy link
Contributor

erenon commented Oct 21, 2024

Currently, recvmsg takes a mut cmsg_buffer: Option<&'a mut Vec<u8>> argument. This potentially forces the caller to allocate a right-sized Vec (or keep one around), though recvmsg implementation immediately produces a slice of the vector, and never uses it as a vector.

What's the reason for having Vec on the interface? Could we have a &mut [u8] instead? That'd allow the called to put the buffer onto the stack.
@asomers
Copy link
Member

asomers commented Oct 21, 2024

If you look at the commit that introduced the &mut Vec argument, 2643afc, you'll see that it did use cmsg_buffer as a Vec, to set its len. It was a later commit, 482a6b1, that removed the set_len operation. So probably the signature can be changed now. Would you care to prepare a PR and test it?

erenon pushed a commit to erenon/nix that referenced this issue Oct 22, 2024
recvmsg: take slice for cmsg_buffer
74e246c 
Instead of a Vec, to avoid forcing an allocation.

`cmsg_space!` now creates a zero initialized vec
(instead of a zero-len vec with capacity):
this way callsites do not need to be changed.

Fixes nix-rust#2523
@erenon erenon mentioned this issue Oct 22, 2024
Merged
@erenon
Copy link
Contributor Author

erenon commented Oct 22, 2024

Possible fix prepared: #2524

github-merge-queue bot pushed a commit that referenced this issue Oct 22, 2024
@erenon
recvmsg: take slice for cmsg_buffer (#2524)
b91bf39 
Instead of a Vec, to avoid forcing an allocation.

`cmsg_space!` now creates a zero initialized vec
(instead of a zero-len vec with capacity):
this way callsites do not need to be changed.

Fixes #2523

Co-authored-by: Benedek Thaler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

recvmsg: take slice for cmsg_buffer erenon/nix
2 participants
@asomers @erenon