You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In reality, users should not be accessing the object directly at all. All access should be abstracted via the MSG API.
Describe the solution you'd like
At the generic API level (abstract type definition), this should really be a struct like the Command and Telemetry headers are. Users of this object should not treat it as anything other an an opaque blob.
Describe alternatives you've considered
N/A
Additional context
Historically, there were two reasons that it was originally a union:
To enforce some type of alignment (e.g. long word) by including an alignment field within the union
To access it as raw bytes/octets for the purpose of e.g. computing a checksum.
Item 1 is obsolete for two reasons:
a) It is already aligned when instantiated as part of a CFE_SB_Buffer_t encapsulating object within SB. There does not need to be alignment in the message type itself
b) Aligning in this way wasn't sufficient and/or didn't meet mission requirements as it constituted compiler padding if it was ever relied upon. In most use cases, structures used for inter-process communication must be specifically designed such that they do not include any padding. For example: if a header was nominally 12 bytes long but needed 64-bit alignment, the accepted approach is to explicitly include a "spare" fields (either uint8[4] or single uint32). It should not be done via this method of allowing the compiler to pad it out.
Item 2 is invalid because users (as in, apps and libraries) should never access the message as raw bytes. The checksum implementation is strictly done by the MSG module itself. Therefore, the MSG module itself (internally) should be the only thing that accesses the message in this manner. Although this could be through a union (and that would be fine) that aspect should be hidden from the public API.
Simply changing this to a struct shouldn't impact any app code, because the typedef stays the same and nothing should be relying on it being a union. (aside from unit tests and the MSG module itself).
Requester Info
Joseph Hickey, Vantage Systems, Inc.
The text was updated successfully, but these errors were encountered:
Pinging @skliper as a heads up. This is a trivial change to the header and only needed minimal updates to UT to make it work. I will be submitting a PR, but wanted to get your attention in case you know of any cases that there is a real need to have this as a union instead of a struct.
Is your feature request related to a problem? Please describe.
The
cfe_msg_api_typedefs.h
file defines theCFE_MSG_Message_t
typedef as a union:cFE/modules/core_api/fsw/inc/cfe_msg_api_typedefs.h
Line 102 in 8cdad66
In reality, users should not be accessing the object directly at all. All access should be abstracted via the MSG API.
Describe the solution you'd like
At the generic API level (abstract type definition), this should really be a
struct
like the Command and Telemetry headers are. Users of this object should not treat it as anything other an an opaque blob.Describe alternatives you've considered
N/A
Additional context
Historically, there were two reasons that it was originally a union:
Item 1 is obsolete for two reasons:
a) It is already aligned when instantiated as part of a
CFE_SB_Buffer_t
encapsulating object within SB. There does not need to be alignment in the message type itselfb) Aligning in this way wasn't sufficient and/or didn't meet mission requirements as it constituted compiler padding if it was ever relied upon. In most use cases, structures used for inter-process communication must be specifically designed such that they do not include any padding. For example: if a header was nominally 12 bytes long but needed 64-bit alignment, the accepted approach is to explicitly include a "spare" fields (either
uint8[4]
or singleuint32
). It should not be done via this method of allowing the compiler to pad it out.Item 2 is invalid because users (as in, apps and libraries) should never access the message as raw bytes. The checksum implementation is strictly done by the MSG module itself. Therefore, the MSG module itself (internally) should be the only thing that accesses the message in this manner. Although this could be through a union (and that would be fine) that aspect should be hidden from the public API.
Simply changing this to a struct shouldn't impact any app code, because the typedef stays the same and nothing should be relying on it being a union. (aside from unit tests and the MSG module itself).
Requester Info
Joseph Hickey, Vantage Systems, Inc.
The text was updated successfully, but these errors were encountered: