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

Add recv_from_into_buf() #246

Closed
wants to merge 1 commit into from
Closed

Add recv_from_into_buf() #246

wants to merge 1 commit into from

Conversation

agrover
Copy link

@agrover agrover commented Jun 30, 2021

It would be nice to have a way to use recv_from without needing MaybeUninit, even if it's a little less efficient.

The code is trivial, IMO the hard part is coming up with a clear name for this method in comparison to recv_from. Any better options?

fixes #223

@Thomasdezeeuw
Copy link
Collaborator

It would be nice to have a way to use recv_from without needing MaybeUninit, even if it's a little less efficient.

Yeah it's a problem I struggled with as well and basically postponed until someone complained, and here we are... TcpStream has it easy as it just implemented the io::Read trait. I don't really want to add &[u8] variants of all functions, as there are many of them for UdpSocket, see recv, recv_out_of_band, etc, having a duplicate of all them using &[u8] is a bit much...

The code is trivial, IMO the hard part is coming up with a clear name for this method in comparison to recv_from. Any better options?

In my higher-level crate I've created a Bytes trait that helps with this, but it's not possible to add a sound implementation that does &[u8] -> &[MaybeUninit<u8>] as that would allow safe code to write uninit bytes to &[u8].

I'm afraid that I don't have a good solution... Could use some more designs/opinions here.

@keepsimple1
Copy link

Regarding the name, how about recv_buf_from ? (I admit it is no longer following recv_from_ prefix, but it also has its advantages, I think)

@fujiapple852
Copy link

@Thomasdezeeuw have any acceptable designs for this emerged since your comment above? cc @agrover

I don't really want to add &[u8] variants of all functions, as there are many of them for UdpSocket, see recv, recv_out_of_band, etc

Is it fair to say this is mostly going to be useful for cases where you require the peer SockAddr (and therefore cannot simply use the Read trait)? Certainly that is the use case that led me here. If so, I think the scope is limited to the following 5 methods, are there any more I have missed?:

  • recv_from
  • recv_from_with_flags
  • recv_from_vectored
  • recv_from_vectored_with_flags
  • peek_from

To enumerate some possible options for discussion purposes:

  • Add _into_buf(&mut [u8]) methods to the Socket impl for the 5 methods above (i.e. this PR, but with other 4 methods added)
  • Add an RecvFromIntoBuf extension trait (which could be enabled via a feature flag) which contains these methods, or perhaps ReadFrom would be a better name since it would be exactly as per the current Read impl except it invokes recv_from rather than recv and returns the SockAddr
  • As per the previous option, but in a separate crate (my current local solution)
  • Some macro magic to create wrapper methods for the 5 methods

@Thomasdezeeuw
Copy link
Collaborator

@fujiapple852 I think the ReadBuf API (std::io::ReadBuf) would be the way forward, but we'll have to wait until it stabilises: rust-lang/rust#78485.

@Thomasdezeeuw
Copy link
Collaborator

Closing in favour of #366.

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.

Socket::recv_from taking a &mut [u8]
4 participants