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

Document that UdpSocket::recv and recv_from do not read from the buffer #657

Closed
dtolnay opened this Issue Aug 18, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@dtolnay
Copy link

dtolnay commented Aug 18, 2017

fn recv(&self, buf: &mut [u8]) -> Result<usize>;
fn recv_from(&self, buf: &mut [u8]) -> Result<(usize, SocketAddr)>;

It is important for these to document that they only write to the buffer, not read from it, because that means it is safe to pass an uninitialized buffer. It would be wasteful to construct a buffer, initialize it to contain all 0 bytes, and pass it to UdpSocket.

@frehberg

This comment has been minimized.

Copy link
Contributor

frehberg commented Aug 30, 2017

And add to the docs, if recv returns with Ok(N), the following code must not iterate over complete array, but iterate over a slice of the array buf[0,..,N-1].

frewsxcv added a commit to frewsxcv/rust that referenced this issue Sep 15, 2017

@frehberg

This comment has been minimized.

Copy link
Contributor

frehberg commented Sep 15, 2017

can be closed?

@dtolnay

This comment has been minimized.

Copy link
Author

dtolnay commented Sep 15, 2017

I don't think so. The docs in mio still need some work.

mio/src/net/udp.rs

Lines 79 to 81 in 5b9d20c

/// Receives data from the socket. On success, returns the number of bytes
/// read and the address from whence the data came.
pub fn recv_from(&self, buf: &mut [u8]) -> io::Result<(usize, SocketAddr)> {

@frehberg

This comment has been minimized.

Copy link
Contributor

frehberg commented Dec 6, 2017

Finally the patch has been rebased, and could be merged

carllerche added a commit that referenced this issue Dec 15, 2017

@carllerche

This comment has been minimized.

Copy link
Owner

carllerche commented Dec 15, 2017

The original PR got messed up somehow. I moved the commit to #775.

carllerche added a commit that referenced this issue Jan 4, 2018

@carllerche

This comment has been minimized.

Copy link
Owner

carllerche commented Jan 4, 2018

Thanks. This has been merged (f6f293f).

@carllerche carllerche closed this Jan 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.