-
Notifications
You must be signed in to change notification settings - Fork 218
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
Fix #88, Add test for name too long and update comments #398
Conversation
Nitpick - could you update the commit message to stand alone? Something like:
|
Oh, and the issue description needs to explicitly say: This autolinks the pull request it with the issue. |
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.
One issue noted in the code, also this needs to be coordinated/preemptively merged with the pull in #405
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.
CCB 20200415 - APPROVED |
Can we add a similar comment to OS_MAX_FILE_NAME or is it too late? |
@dmknutsen @astrogeco actually, I can do the change in my commit... retracting previous comment. |
Describe the contribution
Fixes #88, Add test for name too long and update comments.
Coverage test added to OS_TimerCreate for OS_ERR_NAME_TOO_LONG.
Updated comments related to sizing includes null terminator for:
OS_MAX_API_NAME, OS_MAX_PATH_LEN, OS_BUFFER_SIZE
Testing performed
Ran unit tests.
System(s) tested on
Oracle VM VirtualBox
OS: ubuntu-19.10
Versions: cFE 6.7.11.0, OSAL 5.0.9.0, PSP 1.4.7.0
Contributor Info
Dan Knutsen
NASA/Goddard