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

return for a timeout is negative 4 #970

Closed
excaliburtb opened this issue Apr 26, 2021 · 4 comments · Fixed by #972 or #975
Closed

return for a timeout is negative 4 #970

excaliburtb opened this issue Apr 26, 2021 · 4 comments · Fixed by #972 or #975
Labels
Milestone

Comments

@excaliburtb
Copy link

return_code = OS_GenericRead_Impl(&token, buffer, nbytes, timeout);

Not sure which layer of abstraction to report this but the comment states that a timeout should return 0. However, on Linux/portable BSD... the eventual call to OS_DoSelect is returning OS_ERROR_TIMEOUT (-4) and flowing that all the to the OS_TimedRead API. I would prefer it to be "caught" and returned as 0 at some point but.. more than anything.. just need the comment to match.

@skliper skliper added the bug label Apr 26, 2021
@skliper skliper added this to the 6.0.0 milestone Apr 26, 2021
@jphickey
Copy link
Contributor

Is this the comment you are referring to?

* @return Byte count on success, zero for timeout, or appropriate error code,

Note that a read of size 0 typically indicates an EOF condition. POSIX says this about read():

No  data transfer shall occur past the current end-of-file. If the starting position is at or after the end-of-file, 0 shall be returned.

This is a paradigm that OSAL is borrowing/passing through here, so returning OS_ERROR_TIMEOUT on timeout is preferable in order to distinguish this as a true timeout, rather than EOF. This is particularly important for sockets where the remote end has closed the connection.

I will update the documentation to correct this. We should probably also add a note that a read of size 0 is the EOF indicator.

@excaliburtb
Copy link
Author

yep, the api comments need to not say timeout == 0.

@skliper skliper added docs and removed bug labels Apr 27, 2021
@excaliburtb
Copy link
Author

and just to double-check, double-confirm... for TCP sockets the "replacement" for send and recv calls is OS_TimedRead and OS_TimeWrite, correct?

@jphickey
Copy link
Contributor

Yes, Or regular OS_read/OS_write if you don't need a timeout. The SendTo/RecvFrom abstractions are only intended for datagram sockets. Stream sockets should use either form of read/write to transfer data.

jphickey added a commit to jphickey/osal that referenced this issue Apr 27, 2021
Update the API documentation for OS_read/write/TimedRead/TimedWrite.
Clarify that zero will be returned on EOF condition, and in the
case of the timed API calls, the OS_ERR_TIMEOUT status code will
be returned if the timeout expired without the handle becoming
readable/writable during that time.

This is intended to allow the caller to differentiate between a
handle which is in a state where it cannot read/write data (e.g.
at EOF, or a pipe/socket with remote end closed) and handle which
is simply idle or busy.
@astrogeco astrogeco added the bug label Apr 28, 2021
astrogeco added a commit that referenced this issue Apr 28, 2021
Fix #970, update documentation for read/write
@astrogeco astrogeco removed the bug label Apr 28, 2021
jphickey added a commit to jphickey/osal that referenced this issue Aug 10, 2022
Adjust strncpy call to not trigger warning
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants