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

Fix #742, make sure all pointers are checked for null #766

Merged

Conversation

zanzaben
Copy link
Contributor

@zanzaben zanzaben commented Jan 20, 2021

Describe the contribution
Fixes #742
Went through all the api's and made sure all pointers have a null check, or a comment stating that it can be null.

Fixes #765

Testing performed
Build and run unit test

Expected behavior changes
none

System(s) tested on
Ubuntu 20.04

Additional context
13 To Do left in here that will be fixed by #765

Contributor Info - All information REQUIRED for consideration of pull request
Alex Campbell GSFC

@jphickey
Copy link
Contributor

Note - we should not be doing pointer checks on internal functions - only those that are part of the public API.

I took a glance at the proposed commit and I see it is adding checks to internal routines. This is inefficient and unnecessary - as the value should have been already checked in the public API it was called from. It is also my understanding that code standards only require it on externally-called functions.

@skliper
Copy link
Contributor

skliper commented Jan 21, 2021

Note - we should not be doing pointer checks on internal functions - only those that are part of the public API.

I took a glance at the proposed commit and I see it is adding checks to internal routines. This is inefficient and unnecessary - as the value should have been already checked in the public API it was called from. It is also my understanding that code standards only require it on externally-called functions.

Concur. The coding standard applies to all the APIs.

@zanzaben
Copy link
Contributor Author

After removing all the internal checks this pull request is just a bunch of formatting fixes and changing comments to all use the same terminology.

@skliper
Copy link
Contributor

skliper commented Jan 22, 2021

Can you add the BUGCHECKs in the void public APIs? That would close out the issue.

@zanzaben
Copy link
Contributor Author

Can you add the BUGCHECKs in the void public APIs? That would close out the issue.

Done

@zanzaben zanzaben closed this Jan 22, 2021
@zanzaben zanzaben reopened this Jan 22, 2021
@zanzaben zanzaben changed the title WIP Fix #742, make sure all pointers are checked for null Fix #742, make sure all pointers are checked for null Jan 22, 2021
@zanzaben zanzaben marked this pull request as ready for review January 22, 2021 14:30
@zanzaben zanzaben added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Jan 22, 2021
@astrogeco astrogeco added CCB:2021-01-27 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Jan 27, 2021
@astrogeco
Copy link
Contributor

astrogeco commented Jan 27, 2021

CCB:2021-01-27 APPROVED with CHANGES

OS_ForEachObjectType argument is allowed to be null because it's a passthrough; @jphickey Check documentation for the function to double check this it is well-explained

@astrogeco astrogeco changed the base branch from main to integration-candidate February 3, 2021 22:59
@astrogeco astrogeco merged commit 6787dfe into nasa:integration-candidate Feb 3, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Feb 3, 2021
@zanzaben zanzaben deleted the fix742_null_pointer_cleanup branch February 23, 2021 17:21
@skliper skliper added this to the 6.0.0 milestone Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API's Null pointer check in void methods Scrub API's for null pointer checks
4 participants