Fix solaris unit tests #19
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…uested size exceeds the system maximum value. Also do not reduce the buffer sizes.
|
Before the PR: The unit test hangs part way through. |
|
After the PR: Unit tests run successfully. |
Comment on lines
+269
to
+271
| socklen_t optSize; | ||
| int startRCVBUF = 0; | ||
| optSize = sizeof(startRCVBUF); |
There was a problem hiding this comment.
Suggested change
| socklen_t optSize; | |
| int startRCVBUF = 0; | |
| optSize = sizeof(startRCVBUF); | |
| int startRCVBUF = 0; | |
| socklen_t optSize = sizeof startRCVBUF; |
Comment on lines
+294
to
+298
| if (0 != ::getsockopt( | ||
| m_iSocket, SOL_SOCKET, SO_RCVBUF, (void*)¤tRCVBUF, &optSize)) | ||
| { | ||
| currentRCVBUF = -1; | ||
| } |
There was a problem hiding this comment.
I think this duplicates line 272, doesn't it?
There was a problem hiding this comment.
And you no longer set the option to maxsize?
There was a problem hiding this comment.
Well it may be changed, but return an error. For instance it seems on SunOS it sets to the max allowed system size, and then returns -1 if the requested size could not be set.
Which seems to be >64k, in which case we dont want to reduce the size back to 64k.
|
A new PR submitted to Haivision#2162 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This resolves solaris unit tests failures caused by
srt::CChannel::setUDPSockOpt()callingsetsockopt()which can return failure when attempting to increase the SND|RCV buffers in excess of the system maximums. It seems that since Linux is the odd duck here, this should be the default implementation for all platforms?This also makes sure that we dont try to reduce the size of the SND|RCV buffers as the default is >64k on Solaris and I think most BSD platforms. I assume reducing the size is not a good idea?