Fix #1965, better message type pointer conversion #1966
Merged
Conversation
This file contains 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
skliper
approved these changes
Sep 20, 2021
|
I'm good with the cast too, the only downside is you may only catch real conversion errors when building with strict aliasing and optimization enabled. For users doing active development, this is often not the case - typically code is built in debug mode, no optimizations, and then aliasing violations are typically not warned about. So the macro approach attempts to rectify that, by effectively doing the same member lookup. That being said, if we do go with a macro, I do have to move its definition to actually be part of MSG (in this PR its still part of core_api). |
|
After digging deeper and actually attempting to "force" an error via aliasing rules - reality is it just doesn't work that well (sensitive too compiler settings, compilers warn about things that aren't really violations, and don't warn about things that are). In short - I am now leaning toward keeping the macro, as it can accommodate the EDS/generated header use-case and still provide type safety without relying on compiler features that aren't really that solid. |
|
Sounds good to me, that was my very initial concern when first discussed (how well it's really enforced across compilers, etc). Thanks for doing the analysis, macro works for me. |
jphickey
force-pushed
the
fix-1965-cast-message-types
branch
3 times, most recently
from
bfd5428
to
862665f
Compare
Use a conversion macro, rather than directly referencing a member name/heirarchy within the CFE_MSG header structures, when calling SB APIs that accept a CFE_MSG_Message_t* type. The conversion macro is specific to how the headers are actually defined.
jphickey
force-pushed
the
fix-1965-cast-message-types
branch
from
862665f
to
7440e82
Compare
|
This should be ready, whenever a "post-caelum" merge strategy is identified. For now, I'll merge into a baseline in my fork (because this is necessary for EDS demo). |
This was referenced Sep 30, 2021
astrogeco
added
CCB:Ready
|
CCB:2021-11-03 APPROVED
|
astrogeco
added a commit
to nasa/cFS
that referenced
this pull request
Nov 10, 2021
astrogeco
added a commit
to astrogeco/cFS
that referenced
this pull request
Nov 17, 2021
Combines: nasa/cFE#2001 nasa/osal#1191 nasa/PSP#318 Includes: - nasa/cFE#1663, Use cfe time clock state enum t in cmd/tlm and handling - nasa/cFE#1973, EVS functional test add for different event types - nasa/cFE#1954, Patch for recursive event loop - nasa/cFE#1911, CFE_ES_MemAddOff structure to associated CFE_ES_MemAddress_t and CFE_ES_MemOffset_t - nasa/cFE#1986, Avoid aliasing warnings - nasa/cFE#1975, Message ID type improvements - nasa/cFE#1974, CFE_SB_ValueToMsgId/MsgIdToValue wrappers - nasa/cFE#1663, Use CFE_TIME_ClockState_Enum_t in cmd/tlm and handling - nasa/cFE#1995, Consolidate repeated MSG stub setup in sb_UT - nasa/cFE#1873, Return type conversions in CFE_ES_GetTaskName - nasa/cFE#1966, Better message type pointer conversion - nasa/cFE#2002, Replacing hardcoded message limit in TIME services - nasa/cFE#1909, Use macro in CFE_ResourceId_IsDefined - nasa/cFE#1999, Checking against IsNewOffset only - nasa/osal#1186, Add Duplicate Check to Local Unit Test - nasa/osal#1178, Rename OS_XXXTime to OS_XXXLocalTime in comments - nasa/osal#1168, Fix vxWorks intLib stub aliasing issue - nasa/osal#1169, Recognize ifdef __cplusplus - nasa/osal#1180, Remove extern in stub prototypes - nasa/osal#1112, Add typedef for OSAL status codes - nasa/PSP#301, Sleep before exit when printing - nasa/PSP#314, Check the address in PSP get segment stubs - nasa/elf2cfetbl#89, Check get_sh_entsize(SectionHeader) value - nasa/cFS-GroundSystem#197, Free Address Info for Leak - nasa/sch_lab#95, use separate address variable - nasa/sch_lab#94, Use CFE_MSG_PTR conversion macro - nasa/sample_app#157, Use CFE_MSG_PTR conversion macro - nasa/ci_lab#95, Use CFE_MSG_PTR conversion macro - nasa/to_lab#107, Use CFE_MSG_PTR conversion macro - nasa/elf2cfetbl#93, Check get_sh_entsize(SectionHeader) value Co-authored by: Jacob Hageman [email protected] Co-authored by: Joseph Hickey [email protected] Co-authored by: Ariel Adams [email protected] Co-authored by: Paul [email protected] Co-authored by: Zachary Gonzalez [email protected] Co-authored by: Sam Price [email protected] Co-authored by: Shefali Sharma [email protected]
2 tasks
astrogeco
added a commit
to astrogeco/cFS
that referenced
this pull request
Nov 18, 2021
Combines: nasa/cFE#2001 nasa/osal#1191 nasa/PSP#318 Includes: - nasa/cFE#1663, Use cfe time clock state enum t in cmd/tlm and handling - nasa/cFE#1973, EVS functional test add for different event types - nasa/cFE#1954, Patch for recursive event loop - nasa/cFE#1911, CFE_ES_MemAddOff structure to associated CFE_ES_MemAddress_t and CFE_ES_MemOffset_t - nasa/cFE#1986, Avoid aliasing warnings - nasa/cFE#1975, Message ID type improvements - nasa/cFE#1974, CFE_SB_ValueToMsgId/MsgIdToValue wrappers - nasa/cFE#1663, Use CFE_TIME_ClockState_Enum_t in cmd/tlm and handling - nasa/cFE#1995, Consolidate repeated MSG stub setup in sb_UT - nasa/cFE#1873, Return type conversions in CFE_ES_GetTaskName - nasa/cFE#1966, Better message type pointer conversion - nasa/cFE#2002, Replacing hardcoded message limit in TIME services - nasa/cFE#1909, Use macro in CFE_ResourceId_IsDefined - nasa/cFE#1999, Checking against IsNewOffset only - nasa/osal#1186, Add Duplicate Check to Local Unit Test - nasa/osal#1178, Rename OS_XXXTime to OS_XXXLocalTime in comments - nasa/osal#1168, Fix vxWorks intLib stub aliasing issue - nasa/osal#1169, Recognize ifdef __cplusplus - nasa/osal#1180, Remove extern in stub prototypes - nasa/osal#1112, Add typedef for OSAL status codes - nasa/PSP#301, Sleep before exit when printing - nasa/PSP#314, Check the address in PSP get segment stubs - nasa/elf2cfetbl#89, Check get_sh_entsize(SectionHeader) value - nasa/cFS-GroundSystem#197, Free Address Info for Leak - nasa/sch_lab#95, use separate address variable - nasa/sch_lab#94, Use CFE_MSG_PTR conversion macro - nasa/sample_app#157, Use CFE_MSG_PTR conversion macro - nasa/ci_lab#95, Use CFE_MSG_PTR conversion macro - nasa/to_lab#107, Use CFE_MSG_PTR conversion macro - nasa/elf2cfetbl#93, Check get_sh_entsize(SectionHeader) value Co-authored by: Jacob Hageman <[email protected]> Co-authored by: Joseph Hickey <[email protected]> Co-authored by: Ariel Adams <[email protected]> Co-authored by: Paul <[email protected]> Co-authored by: Zachary Gonzalez <[email protected]> Co-authored by: Sam Price <[email protected]> Co-authored by: Shefali Sharma <[email protected]>
astrogeco
added a commit
to astrogeco/cFS
that referenced
this pull request
Nov 18, 2021
Combines: nasa/cFE#2001 nasa/osal#1191 nasa/PSP#318 Includes: - nasa/cFE#1663, Use cfe time clock state enum t in cmd/tlm and handling - nasa/cFE#1973, EVS functional test add for different event types - nasa/cFE#1954, Patch for recursive event loop - nasa/cFE#1911, CFE_ES_MemAddOff structure to associated CFE_ES_MemAddress_t and CFE_ES_MemOffset_t - nasa/cFE#1986, Avoid aliasing warnings - nasa/cFE#1975, Message ID type improvements - nasa/cFE#1974, CFE_SB_ValueToMsgId/MsgIdToValue wrappers - nasa/cFE#1663, Use CFE_TIME_ClockState_Enum_t in cmd/tlm and handling - nasa/cFE#1995, Consolidate repeated MSG stub setup in sb_UT - nasa/cFE#1873, Return type conversions in CFE_ES_GetTaskName - nasa/cFE#1966, Better message type pointer conversion - nasa/cFE#2002, Replacing hardcoded message limit in TIME services - nasa/cFE#1909, Use macro in CFE_ResourceId_IsDefined - nasa/cFE#1999, Checking against IsNewOffset only - nasa/osal#1186, Add Duplicate Check to Local Unit Test - nasa/osal#1178, Rename OS_XXXTime to OS_XXXLocalTime in comments - nasa/osal#1168, Fix vxWorks intLib stub aliasing issue - nasa/osal#1169, Recognize ifdef __cplusplus - nasa/osal#1180, Remove extern in stub prototypes - nasa/osal#1112, Add typedef for OSAL status codes - nasa/PSP#301, Sleep before exit when printing - nasa/PSP#314, Check the address in PSP get segment stubs - nasa/elf2cfetbl#89, Check get_sh_entsize(SectionHeader) value - nasa/cFS-GroundSystem#197, Free Address Info for Leak - nasa/sch_lab#95, use separate address variable - nasa/sch_lab#94, Use CFE_MSG_PTR conversion macro - nasa/sample_app#157, Use CFE_MSG_PTR conversion macro - nasa/ci_lab#95, Use CFE_MSG_PTR conversion macro - nasa/to_lab#107, Use CFE_MSG_PTR conversion macro - nasa/elf2cfetbl#93, Check get_sh_entsize(SectionHeader) value Co-authored-by: Jacob Hageman <[email protected]> Co-authored-by: Joseph Hickey <[email protected]> Co-authored-by: Ariel Adams <[email protected]> Co-authored-by: Paul <[email protected]> Co-authored-by: Zachary Gonzalez <[email protected]> Co-authored-by: Sam Price <[email protected]> Co-authored-by: Shefali Sharma <[email protected]>
astrogeco
added a commit
to astrogeco/cFS
that referenced
this pull request
Nov 18, 2021
Combines: nasa/cFE#2001 nasa/osal#1191 nasa/PSP#318 Includes: - nasa/cFE#1663, Use cfe time clock state enum t in cmd/tlm and handling - nasa/cFE#1973, EVS functional test add for different event types - nasa/cFE#1954, Patch for recursive event loop - nasa/cFE#1911, CFE_ES_MemAddOff structure to associated CFE_ES_MemAddress_t and CFE_ES_MemOffset_t - nasa/cFE#1986, Avoid aliasing warnings - nasa/cFE#1975, Message ID type improvements - nasa/cFE#1974, CFE_SB_ValueToMsgId/MsgIdToValue wrappers - nasa/cFE#1663, Use CFE_TIME_ClockState_Enum_t in cmd/tlm and handling - nasa/cFE#1995, Consolidate repeated MSG stub setup in sb_UT - nasa/cFE#1873, Return type conversions in CFE_ES_GetTaskName - nasa/cFE#1966, Better message type pointer conversion - nasa/cFE#2002, Replacing hardcoded message limit in TIME services - nasa/cFE#1909, Use macro in CFE_ResourceId_IsDefined - nasa/cFE#1999, Checking against IsNewOffset only - nasa/osal#1186, Add Duplicate Check to Local Unit Test - nasa/osal#1178, Rename OS_XXXTime to OS_XXXLocalTime in comments - nasa/osal#1168, Fix vxWorks intLib stub aliasing issue - nasa/osal#1169, Recognize ifdef __cplusplus - nasa/osal#1180, Remove extern in stub prototypes - nasa/osal#1112, Add typedef for OSAL status codes - nasa/PSP#301, Sleep before exit when printing - nasa/PSP#314, Check the address in PSP get segment stubs - nasa/elf2cfetbl#89, Check get_sh_entsize(SectionHeader) value - nasa/cFS-GroundSystem#197, Free Address Info for Leak - nasa/sch_lab#95, use separate address variable - nasa/sch_lab#94, Use CFE_MSG_PTR conversion macro - nasa/sample_app#157, Use CFE_MSG_PTR conversion macro - nasa/ci_lab#95, Use CFE_MSG_PTR conversion macro - nasa/to_lab#107, Use CFE_MSG_PTR conversion macro - nasa/elf2cfetbl#93, Check get_sh_entsize(SectionHeader) value Co-authored-by: Jacob Hageman <[email protected]> Co-authored-by: Joseph Hickey <[email protected]> Co-authored-by: Ariel Adams <[email protected]> Co-authored-by: Paul <[email protected]> Co-authored-by: Zachary Gonzalez <[email protected]> Co-authored-by: Sam Price <[email protected]> Co-authored-by: Shefali Sharma <[email protected]>
astrogeco
added a commit
to astrogeco/cFS
that referenced
this pull request
Nov 18, 2021
Combines: nasa/cFE#2001 nasa/osal#1191 nasa/PSP#318 Includes: - nasa/cFE#1663, Use cfe time clock state enum t in cmd/tlm and handling - nasa/cFE#1973, EVS functional test add for different event types - nasa/cFE#1954, Patch for recursive event loop - nasa/cFE#1911, CFE_ES_MemAddOff structure to associated CFE_ES_MemAddress_t and CFE_ES_MemOffset_t - nasa/cFE#1986, Avoid aliasing warnings - nasa/cFE#1975, Message ID type improvements - nasa/cFE#1974, CFE_SB_ValueToMsgId/MsgIdToValue wrappers - nasa/cFE#1663, Use CFE_TIME_ClockState_Enum_t in cmd/tlm and handling - nasa/cFE#1995, Consolidate repeated MSG stub setup in sb_UT - nasa/cFE#1873, Return type conversions in CFE_ES_GetTaskName - nasa/cFE#1966, Better message type pointer conversion - nasa/cFE#2002, Replacing hardcoded message limit in TIME services - nasa/cFE#1909, Use macro in CFE_ResourceId_IsDefined - nasa/cFE#1999, Checking against IsNewOffset only - nasa/osal#1186, Add Duplicate Check to Local Unit Test - nasa/osal#1178, Rename OS_XXXTime to OS_XXXLocalTime in comments - nasa/osal#1168, Fix vxWorks intLib stub aliasing issue - nasa/osal#1169, Recognize ifdef __cplusplus - nasa/osal#1180, Remove extern in stub prototypes - nasa/osal#1112, Add typedef for OSAL status codes - nasa/PSP#301, Sleep before exit when printing - nasa/PSP#314, Check the address in PSP get segment stubs - nasa/elf2cfetbl#89, Check get_sh_entsize(SectionHeader) value - nasa/cFS-GroundSystem#197, Free Address Info for Leak - nasa/sch_lab#95, use separate address variable - nasa/sch_lab#94, Use CFE_MSG_PTR conversion macro - nasa/sample_app#157, Use CFE_MSG_PTR conversion macro - nasa/ci_lab#95, Use CFE_MSG_PTR conversion macro - nasa/to_lab#107, Use CFE_MSG_PTR conversion macro - nasa/elf2cfetbl#93, Check get_sh_entsize(SectionHeader) value Co-authored-by: Jacob Hageman <[email protected]> Co-authored-by: Joseph Hickey <[email protected]> Co-authored-by: Ariel Adams <[email protected]> Co-authored-by: Paul <[email protected]> Co-authored-by: Zachary Gonzalez <[email protected]> Co-authored-by: Sam Price <[email protected]> Co-authored-by: Shefali Sharma <[email protected]>
astrogeco
added a commit
to nasa/cFS
that referenced
this pull request
Nov 18, 2021
Combines: nasa/cFE#2001 nasa/osal#1191 nasa/PSP#318 Includes: - nasa/cFE#1663, Use cfe time clock state enum t in cmd/tlm and handling - nasa/cFE#1973, EVS functional test add for different event types - nasa/cFE#1954, Patch for recursive event loop - nasa/cFE#1911, CFE_ES_MemAddOff structure to associated CFE_ES_MemAddress_t and CFE_ES_MemOffset_t - nasa/cFE#1986, Avoid aliasing warnings - nasa/cFE#1975, Message ID type improvements - nasa/cFE#1974, CFE_SB_ValueToMsgId/MsgIdToValue wrappers - nasa/cFE#1663, Use CFE_TIME_ClockState_Enum_t in cmd/tlm and handling - nasa/cFE#1995, Consolidate repeated MSG stub setup in sb_UT - nasa/cFE#1873, Return type conversions in CFE_ES_GetTaskName - nasa/cFE#1966, Better message type pointer conversion - nasa/cFE#2002, Replacing hardcoded message limit in TIME services - nasa/cFE#1909, Use macro in CFE_ResourceId_IsDefined - nasa/cFE#1999, Checking against IsNewOffset only - nasa/osal#1186, Add Duplicate Check to Local Unit Test - nasa/osal#1178, Rename OS_XXXTime to OS_XXXLocalTime in comments - nasa/osal#1168, Fix vxWorks intLib stub aliasing issue - nasa/osal#1169, Recognize ifdef __cplusplus - nasa/osal#1180, Remove extern in stub prototypes - nasa/osal#1112, Add typedef for OSAL status codes - nasa/PSP#301, Sleep before exit when printing - nasa/PSP#314, Check the address in PSP get segment stubs - nasa/elf2cfetbl#89, Check get_sh_entsize(SectionHeader) value - nasa/cFS-GroundSystem#197, Free Address Info for Leak - nasa/sch_lab#95, use separate address variable - nasa/sch_lab#94, Use CFE_MSG_PTR conversion macro - nasa/sample_app#157, Use CFE_MSG_PTR conversion macro - nasa/ci_lab#95, Use CFE_MSG_PTR conversion macro - nasa/to_lab#107, Use CFE_MSG_PTR conversion macro - nasa/elf2cfetbl#93, Check get_sh_entsize(SectionHeader) value Co-authored-by: Jacob Hageman <[email protected]> Co-authored-by: Joseph Hickey <[email protected]> Co-authored-by: Ariel Adams <[email protected]> Co-authored-by: Paul <[email protected]> Co-authored-by: Zachary Gonzalez <[email protected]> Co-authored-by: Sam Price <[email protected]> Co-authored-by: Shefali Sharma <[email protected]>
astrogeco
added a commit
to astrogeco/cFS
that referenced
this pull request
Nov 18, 2021
Combines: nasa/cFE#2001 nasa/osal#1191 nasa/PSP#318 nasa/ci_lab#101 nasa/sch_lab#102 nasa/to_lab#112 nasa/sample_app#163 nasa/cFS-GroundSystem#201 nasa/elf2cfetbl#98 Includes: *cFE* - nasa/cFE#1663, Use cfe time clock state enum t in cmd/tlm and handling - nasa/cFE#1973, EVS functional test add for different event types - nasa/cFE#1954, Patch for recursive event loop - nasa/cFE#1911, CFE_ES_MemAddOff structure to associated CFE_ES_MemAddress_t and CFE_ES_MemOffset_t - nasa/cFE#1986, Avoid aliasing warnings - nasa/cFE#1975, Message ID type improvements - nasa/cFE#1974, CFE_SB_ValueToMsgId/MsgIdToValue wrappers - nasa/cFE#1663, Use CFE_TIME_ClockState_Enum_t in cmd/tlm and handling - nasa/cFE#1995, Consolidate repeated MSG stub setup in sb_UT - nasa/cFE#1873, Return type conversions in CFE_ES_GetTaskName - nasa/cFE#1966, Better message type pointer conversion - nasa/cFE#2002, Replacing hardcoded message limit in TIME services - nasa/cFE#1909, Use macro in CFE_ResourceId_IsDefined - nasa/cFE#1999, Checking against IsNewOffset only *osal* - nasa/osal#1186, Add Duplicate Check to Local Unit Test - nasa/osal#1178, Rename OS_XXXTime to OS_XXXLocalTime in comments - nasa/osal#1168, Fix vxWorks intLib stub aliasing issue - nasa/osal#1169, Recognize ifdef __cplusplus - nasa/osal#1180, Remove extern in stub prototypes - nasa/osal#1112, Add typedef for OSAL status codes *PSP* - nasa/PSP#301, Sleep before exit when printing - nasa/PSP#314, Check the address in PSP get segment stubs *Tools* - nasa/elf2cfetbl#89, Check get_sh_entsize(SectionHeader) value - nasa/elf2cfetbl#93, Check get_sh_entsize(SectionHeader) value - nasa/cFS-GroundSystem#197, Free Address Info for Leak *Apps* - nasa/sch_lab#95, use separate address variable - nasa/sch_lab#94, Use CFE_MSG_PTR conversion macro - nasa/sample_app#157, Use CFE_MSG_PTR conversion macro - nasa/ci_lab#95, Use CFE_MSG_PTR conversion macro - nasa/to_lab#107, Use CFE_MSG_PTR conversion macro Co-authored-by: Jacob Hageman <[email protected]> Co-authored-by: Joseph Hickey <[email protected]> Co-authored-by: Ariel Adams <[email protected]> Co-authored-by: Paul <[email protected]> Co-authored-by: Zachary Gonzalez <[email protected]> Co-authored-by: Sam Price <[email protected]> Co-authored-by: Shefali Sharma <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Provides more abstract conversions between the local buffer pointers and the
CFE_MSG_Message_t*pointer required for calling CFE_SB API, without assuming specific member names.Fixes #1965
Testing performed
Build and sanity check CFE, run all tests
Expected behavior changes
None in framework build.
Improves source code compatibility when using an alternate MSG implementation that does not (directly) contain a
Msgmember.System(s) tested on
Ubuntu
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.