search

nasa / CF

The Core Flight System (cFS) CFDP application.
Apache License 2.0
79 stars 45 forks source link

CF C99 compliance and use of packed structures #41

Closed jphickey closed 2 years ago

jphickey commented 2 years ago

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1777] CF C99 compliance and use of packed structures Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 15:53:48 2021

Original Description: CF currently uses the 'CF_PACK` attribute on many of its structure types, which translates to the gcc "__attribute__((packed))" extension, without any provision for other compilers.

This is a nonstandard/compiler-specific extension feature and makes the code NOT c99-compliant. Attempting to build this code with non-gcc compiler will likely fail as a result.

Application source code should be limited to standard C99 features, and should not rely on vendor-specific extensions.

jphickey commented 2 years ago

Imported comment by internal user on Tue Nov 16 15:57:15 2021

It should be possible to be compliant without the use of packed structures. To do so, the data structures should be defined only using uint8-based fields. Instead of using larger (uint16/32) integer types they should be declared as an array of uint8 of the correct size. Since these are not directly usable fields as it is (all access needs to go through a byte-order/endianness conversion anyway) this difference should not be significant, and can likely be compensated by updating only those wrappers that do the endianness conversions.

jphickey commented 2 years ago

There are two separate aspects here:

  1. Commands and Telemetry for ground control of the CF app itself. These are application-defined structures and do not need to specifically to any specific standard, other than what CFE SB + CFE MSG dictate for the encapsulation. Therefore these can be aligned however necessary. It is not required to pack these structs at all - we can simply remove the packing here. Note that they are already laid out in such a way that implicit padding is not likely to be an issue, so removing the packed attribute probably won't even change them. And also note that no other CFS apps pack their command/telemetry structures right now, either.

  2. CFDP PDUs. These are required to conform to the CCSDS 727.0-B-5 definition and therefore must be aligned according to that spec, not aligned per the CPU. But these are also encoded per CCSDS 727.0 as well, so they aren't directly usable as logical values anyway. Recommend to change these to uint8-based structs so the packed attribute will not be needed.

The fix for this should be split per above, since these are really separate problem domains. The PR for this issue will just focus on the CMD/TLM (1) above. Since the CFDP PDU decoding is more involved and has other problems, those will be fixed in the context of issues #59, #71, and #116.