-
Notifications
You must be signed in to change notification settings - Fork 3k
Clarify TCPSocket::recv() and UDPSocket::recvfrom() documentation. #5623
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
Conversation
features/netsocket/UDPSocket.h
Outdated
* non-blocking or times out, NSAPI_ERROR_WOULD_BLOCK is returned | ||
* immediately. | ||
* | ||
* @param address Destination for the source address or NULL | ||
* @param data Destination buffer for data received from the host | ||
* @param size Size of the buffer in bytes | ||
* @return Number of received bytes on success, negative error | ||
* @return Size of received datagram, negative error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, this wording is wrong in a different way - if the datagram is bigger than your buffer, it should return the bytes actually read - it's quietly truncated. (Which means applications have to provide known-to-be-big-enough buffers for whatever protocol they're implementing).
The body is too wooly about "receiving data", but at least the brief is already correct, I see - it does say "Receive a packet", compared to "Receive data" on TCPSocket::recv
.
How about transforming to "datagram" everywhere:
/** Receive a datagram over a UDP socket
*
* Receives a datagram and stores the source address in address if address
* is not NULL. Returns the number of bytes written into the buffer. If the
* datagram is larger than the buffer, the excess data is silently discarded.
*
* By default, recvfrom blocks until a datagram is received. If socket is set to
* non-blocking or times out with no datagram, NSAPI_ERROR_WOULD_BLOCK
* is returned.
*
* @param address Destination for the source address or NULL
* @param data Destination buffer for datagram received from the host
* @param size Size of the buffer in bytes
* @return Number of received bytes on success, negative error
* code on failure
*/
699e97b
to
6ad6ff7
Compare
Updated the UDPSocket::recvfrom() documentation as Kevin suggested. I was not sure about the truncating so I left it out originally. Good that it is now specified. |
features/netsocket/TCPSocket.h
Outdated
@@ -111,7 +111,9 @@ class TCPSocket : public Socket { | |||
* @param data Destination buffer for data received from the host | |||
* @param size Size of the buffer in bytes | |||
* @return Number of received bytes on success, negative error | |||
* code on failure | |||
* code on failure. If no messages are available to be received |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conversely to the UDP case, this should be "no [more?] data is available", rather than introducing "messages".
features/netsocket/TCPSocket.h
Outdated
* code on failure | ||
* code on failure. If no messages are available to be received | ||
* and the peer has performed an orderly shutdown, | ||
* recv() shall return 0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"returns 0" - everything else is present tense.
@AnotherButler Docs! |
* By default, recvfrom blocks until data is sent. If socket is set to | ||
* non-blocking or times out, NSAPI_ERROR_WOULD_BLOCK is returned | ||
* immediately. | ||
* By default, recvfrom blocks until a datagram is received. If socket is set to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Query: Who or what receives the datagram? Please clarify for active voice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about "blocks until a datagram arrives"?
(POSIX version is much more wordy and specific, with a couple of "if no messages are available..." Don't know if it's worth spelling that out. Could compromise with "has arrived", to imply it may have already arrived before the call.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like "blocks until a datagram arrives".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see much difference between "blocks until a datagram is received" and "blocks until a datagram arrives".
It might even confuse more. Datagram can arrive earlier that user calls this function. Then user receives the datagram from the network stack's buffer. So "datagram arrives" is not totally accurate. Should we clarify this a bit? I found it already clear for me, so I assume SW engineer understand accurately it already.
For a comparison, POSIX standard definition of this same function http://pubs.opengroup.org/onlinepubs/009695399/functions/recvfrom.html
And as this function is recvfrom()
then the answer to "who receives" is just who ever calls this function.
Query: Do we need to update or add to any of the text on either of these pages? https://os.mbed.com/docs/v5.7/reference/tcpsocket.html The actual class reference is transcluded, so it will change automatically. Does any of the rest of it need changing? |
@AnotherButler Who were you asking? |
@cmonr Anyone who feels confident answering. |
@AnotherButler I think those pages should be fine as long as this PR is transcluded when rebuilt. |
@cmonr Awesome, thanks 👍 |
/morph build |
Build : SUCCESSBuild number : 845 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 513 |
Test : SUCCESSBuild number : 689 |
6ad6ff7
to
6bf0611
Compare
I updated the TCPSocket documentation based on Kevin's feedback. |
/morph build |
Build : SUCCESSBuild number : 861 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 532 |
Test : SUCCESSBuild number : 707 |
recv()
call to return zero on end-of-stream.recvfrom()
to actually return size of datagram, not some arbitrary size.CC: @kjbracey-arm @sg-