cFS-Caelum Review, CFS-42: SB, MSG, and Globals

[astrogeco]

Description

This is a "bookkeeping" Pull Request meant for the cFS-Caelum Review code inspection (full-scale code review).

This PR is solely focused on "CFS-42". For more info see this readme

The Included files are

# Software Bus (SB)
!modules/core_api/fsw/inc/cfe_sb*

!modules/core_private/fsw/inc/cfe_sb*

!modules/sb/fsw/**
!modules/sb/CMakeLists.txt

!modules/sbr/fsw/src/cfe_sbr**
!modules/sbr/CMakeLists.txt

# CCSDS Message (MSG)
!modules/core_api/fsw/inc/cfe_msg*

!modules/msg/fsw/**
!modules/msg/option_inc/**
!modules/msg/CMakeLists.txt

Objectives

This review starts on 04/19/2021 and ends on 04/23/2021.
2. Dispositions of findings is on 04/26/2021.

3. Reviewers only need to review source files, header files & build files.

4. Use .ppt, .pdf, .txt & .xlsx files for background information about the code.

5. See the Attachments section for Peer Review Data Package.

6. See also "The Power of 10" rules for safety-critical code. https://en.wikipedia.org/wiki/The_Power_of_10:_Rules_for_Developing_Safety-Critical_Code#:~:text=The%20Power%20of%2010%20Rules,to%20review%20or%20statically%20analyze

7. NOTE: Don't spend too much time over coding standard violations. The Static Code Analysis tool will enforce the coding standards. This code is developed by GSFC, so GSFC coding standards will be enforced for this code base.

Notes

Note a few already existing issues (no need to individually comment on occurrences):

Doxygen event documentation doesn't match code: #508
End of function comments out of date (generalized/paraphrased version of #275)
Update code/unit tests to use CFE_Status_t: #921

If there's anything else that is observed as a repeated pattern, feel free to document as a general comment

There are several places that would trigger warnings with some common compilers/warning options. It would be nice to follow #10 rule in "The Power of 10".

Quick summary/references for currently enforced settings on the FSW
Compiler options (note -Wall and -Werror) -

cFE/cmake/sample_defs/arch_build_custom.cmake

Lines 28 to 37 in 0850b19

	add_compile_options(
	-std=c99 # Target the C99 standard (without gcc extensions)
	-pedantic # Issue all the warnings demanded by strict ISO C
	-Wall # Warn about most questionable operations
	-Wstrict-prototypes # Warn about missing prototypes
	-Wwrite-strings # Warn if not treating string literals as "const"
	-Wpointer-arith # Warn about suspicious pointer operations
	-Wcast-align # Warn about casts that increase alignment requirements
	-Werror # Treat warnings as errors (code should be clean)
	)

cppcheck -

cFE/.github/workflows/run_fsw_cppcheck.sh

Line 3 in 0850b19

cppcheck_common_opts="--force --inline-suppr --std=c99 --language=c --enable=warning,performance,portability,style --suppress=variableScope --inconclusive"

CodeQL -

cFE/.github/workflows/codeql-build.yml

Lines 35 to 39 in 0850b19

	- name: Initialize CodeQL
	uses: github/codeql-action/init@v1
	with:
	languages: c
	queries: +security-extended, security-and-quality

CodeSonar - currently using default set for cFE, extending to JPL and MISRA is future work

For CodeQL and CodeSonar we don't eliminate all warnings, but we do analyze and disposition them all (plan to report dispositions as part of certification package)

This approach is compliant with the latest GSFC 582 standard (that is still going through review). Happy to discuss any additional settings that you have concerns about.

[skliper]

Review closed 4/26. For any future comments please open a new issue.