Fix #337, fix memory corruption produced by misplaced memset()#338
Merged
astrogeco merged 1 commit intonasa:ic-20200226from Feb 25, 2020
Merged
Conversation
Calling memset too late after assigning the `sem` variable in the code of `OS_BinSemCreate_Impl` caused the data pointed to by `sem` to get corrupted. The issue was not caught by the existing test suite for POSIX OSAL when running on Linux. However running the test suite on macOS revealed the anomalies in the behavior of `pthread_cond_destroy()` which was working on the corrupted memory as was demonstrated in: Calling pthread_cond_destroy results in “Function not implemented” ENOSYS on macOS, https://stackoverflow.com/questions/59560940/calling-pthread-cond-destroy-results-in-function-not-implemented-enosys-on-mac?noredirect=1#comment105301077_59560940 The original commit that introduced the issue is: nasa@bfa7a33
jphickey
approved these changes
Jan 2, 2020
Contributor
jphickey
left a comment
jphickey
left a comment
There was a problem hiding this comment.
Tested on my Linux dev machine (Ubuntu 18.04 LTS 64 bit) and confirmed everything works as expected.
14 tasks
skliper
approved these changes
Jan 22, 2020
Contributor
Author
|
@skliper I have just signed the individual CLA and sent it to the email address specified in the document. |
Contributor
|
@stanislaw could you indicate if this is your personal contribution or as part of work for your company? I need to reference the appropriate CLA for our process. If your other pull requests are different (personal vs company or vice versa), could you indicate there also? Thanks. |
Contributor
Author
|
I have just edited the description of this PR. It is individual and I have signed the ICLA and sent it yesterday. |
astrogeco
added
the
CCB:Ready
astrogeco
changed the title
ghost
approved these changes
Feb 11, 2020
Contributor
|
CCB 20200212 - Approved |
astrogeco
added
CCB:Approved
Contributor
Author
|
Just a reminder: the individual CLA has been signed on this one. See here. |
skliper
removed
the
CCB:PendingCLA
Merged
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Describe the contribution
This PR fixes #337. The exact change has been recommended by @jphickey.
Testing performed
Currently, I cannot test this on Linux only on macOS. As reported in the issue, I confirm that that the issue gets fixed with this change applied. Also, the bug has been confirmed by @jphickey and the changeset is created from his recommendation.
Expected behavior changes
See related issue: #337.
System(s) tested on
Contributor Info - All information REQUIRED for consideration of pull request
Stanislav Pankevich, personal
The signed individual CLA has been sent to the email specified in the CLA document.