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

Missing some dependency include files in public API headers #1942

Closed
jphickey opened this issue Sep 9, 2021 · 0 comments · Fixed by #1943 or #1967
Closed

Missing some dependency include files in public API headers #1942

jphickey opened this issue Sep 9, 2021 · 0 comments · Fixed by #1943 or #1967
Assignees
Labels
Milestone

Comments

@jphickey
Copy link
Contributor

jphickey commented Sep 9, 2021

Describe the bug
As a general rule of thumb, all header files should directly include whatever dependencies they require in order to provide the types/declarations they intend to provide.

However in the CFE headers there remain a couple omissions/mistakes in this regard:

  • cfe_es.h declares the function CFE_ES_GetModuleInfo which accepts a resource ID input, so this depends on cfe_resourceid_api_typedefs.h, but it does not directly include this dependency
  • cfe_tbl_api_typedefs.h defines a CFE_TBL_Info_t type, which in turn has a member sized to CFE_MISSION_MAX_PATH_LEN, which is provided by cfe_mission_cfg.h, but it does not directly include this dependency.

In both cases the current framework sample builds do compile successfully, because the dependent header gets included implicitly (i.e. by some header before it) in all the current use cases, but this could change in other use cases.

To Reproduce
Use header files in contexts beyond what the current framework does, such as 3rd party code only including "cfe_es.h" or "cfe_tbl_api_typedefs.h" directly.

Expected behavior
Headers should work (compile w/o errors or warnings) when included individually, they should include all dependencies directly rather than relying on inclusion ordering.

System observed on:
Ubuntu

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Sep 9, 2021
@jphickey jphickey added the bug label Sep 9, 2021
@jphickey jphickey changed the title Add missing include files Missing some dependency include files in public API headers Sep 9, 2021
jphickey added a commit to jphickey/cFE that referenced this issue Sep 9, 2021
Some CFE API headers were missing dependency inclusions, where the header
was referencing a type or symbol but not directly including the header
file that provides that type/symbol.

Adding the dependent include allows the headers to work more consistently.
astrogeco added a commit that referenced this issue Sep 21, 2021
Fix #1942, add missing inclusions in CFE API headers
@skliper skliper added this to the 7.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
Projects
None yet
2 participants