Skip to content

/mio

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 buff… #740

Closed
wants to merge 1 commit into from

Conversation

Reviewers

@KodrAus KodrAus

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@frehberg @carllerche @KodrAus
@frehberg
Copy link
Contributor

frehberg commented Oct 3, 2017

…er (#657)
@KodrAus

KodrAus reviewed Oct 4, 2017
View changes

Copy link

KodrAus left a comment

Thanks for doing this @frehberg! I've just left a few small comments that apply to both doc additions.
src/udp.rs Outdated
/// excess bytes may be discarded.
///
/// The function is overwriting previous content of `buf`. Slicing
/// `&buf[..NumRead]` allows efficient access and boundary checking at compile time.

This comment has been minimized.

Sign in to view
Copy link
@KodrAus

KodrAus Oct 4, 2017

Assuming NumRead is the result of recv_from I think a more idiomatic name for it is num_read, so we know it's some local variable.
src/udp.rs Outdated
@@ -82,6 +82,13 @@ impl UdpSocket {

/// Receives data from the socket. On success, returns the number of bytes
/// read and the address from whence the data came.
///
/// The function must be called with valid byte array `buf` of sufficient size to

This comment has been minimized.

Sign in to view
Copy link
@KodrAus

KodrAus Oct 4, 2017

I think we still need to mention that this function only writes into the buffer and never reads from it.
src/udp.rs Outdated
/// the number of bytes read and the address from whence the data came.
/// the number of bytes read.
///
/// The function must be called with valid byte array `buf` of sufficient size to

This comment has been minimized.

Sign in to view
Copy link
@KodrAus

KodrAus Oct 4, 2017

I think we still need to mention that this function only writes into the buffer and never reads from it.
@frehberg

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

frehberg commented Oct 4, 2017

API-Doc has been extended, adding these aspects. I had to re-arrange the text a bit, does it please?
@carllerche

This comment has been minimized.

Sign in to view
Copy link
Owner

carllerche commented Oct 5, 2017

Thanks... this looks good to me. Unfortunately, it looks like Android CI broke... which going to block me from merging until it is fixed 😢
@carllerche

This comment has been minimized.

Sign in to view
Copy link
Owner

carllerche commented Oct 11, 2017

A CI fix has been merged to master. Would you mind rebasing when you have a moment? Thanks!
@KodrAus

This comment has been minimized.

Sign in to view
Copy link

KodrAus commented Nov 8, 2017

@frehberg How's it going? Would you like a hand rebasing this?
@carllerche

This comment has been minimized.

Sign in to view
Copy link
Owner

carllerche commented Dec 15, 2017

Something wonky happened with this PR. The commit was rebased, but didn't make it to the PR. I moved it here: #775

@carllerche carllerche closed this Dec 15, 2017

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.