Use 0 as CFE_SB_INVALID_MSG_ID

[jphickey]

Is your feature request related to a problem? Please describe.
Currently the value for an "invalid" MSG ID is -1, as defined here:

cFE/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h

Line 86 in e5d4ed9

#define CFE_SB_MSGID_RESERVED CFE_SB_MSGID_WRAP_VALUE(-1)

The pattern used in other modules (which is preferred) is to use 0 as the invalid value, for several reasons:

• Local variables/structures which are explicitly memset to 0 before use (common/recommended practice) will get automatically set to the INVALID value
• Global data structures in BSS section get automatically memset to 0 by the loader
• Uninitialized members of a partially initialized data structure will be automatically memset to 0

So, its much safer to embrace 0 as the reserved/invalid/placeholder value, due to all the different ways memory is cleared to 0 both implicitly and explicitly.

Describe the solution you'd like
Change the definition of CFE_SB_INVALID_MSG_ID to be 0, rather than -1.

Additional context
Standard headers (historical/v1) are safe because any valid MsgId always has the "secondary header" flag set (bit 11). So any valid MsgId is already guaranteed to be nonzero. Should be trivial to change in this config.

Will require a check/confirmation of the Extended headers (v2) configuration, to ensure that the MsgId value 0 does not correlate to a valid address. Since one assumes that 0xFFFFFFFF (-1) already does not correlate to a valid address, it may be as simple as just flipping the bits or adding 1, if that's an issue.

Note that most other resource types (AppId, TaskId, OSAL IDs, MemHandle, etc) already use 0 as the invalid/reserved value for the same reason. MsgId and TableId are still outliers that do not do this. For consistency and reliability reasons they should both be updated. (TableId can be fixed under a separate ticket, possibly as part of a more complete refactor of TBL services)

Requester Info
Joseph Hickey, Vantage Systems, Inc.

[jphickey]

Quick check of the default "v2" msgid implementation, shows that all 16 bits are used, and the secondary header flag is not one of them:

 * Message Id:
 *   7 bits from the primary header APID (0x7F of 0x7FF)
 *   1 bit for the command/telemetry flag
 *   0 bits from the Playback flag
 *   8 bits from the secondary header APID qualifier (Subsystem) (0xFF of 0x01FF)
 *   0 bits from the secondary header APID qualifier as the System
 *   = 16 bits total
 *
 *              Byte 1              Byte 0
 *        7 6 5 4 3 2 1 0     7      6 5 4 3 2 1 0
 *       +-+-+-+-+-+-+-+-+|--------|+-+-+-+-+-+-+-+
 *       | APID Qualifier |C/T flg | Pri Hdr APID |
 *       +-+-+-+-+-+-+-+-+|--------|+-+-+-+-+-+-+-+

So in short, there is no protection against a valid "0" here, but neither is there protection against a valid "0xFFFF" here either. So it wouldn't be much different to use 0 rather than -1 as the reserved value, there would still be one MsgId value that the system designer/integrator must avoid. In that sense, its probably easier to document avoiding 0 than 0xFFFF.

[skliper]

but neither is there protection against a valid "0xFFFF" here either.

But 0xFFFFFFFF is the invalid MsgID value currently, not 0xFFFF. V2 can never possibly return 0xFFFFFFFF so I don't really follow the argument here. The problem is the MsgID parts in a header can be all zero in some cases based on the way it's currently designed... As you mention there are other parts that can't be zero, but MsgID isn't one of them. Even for V1, a 0 isn't technically invalid, it just means there isn't a secondary header (still a perfectly valid CCSDS message). In fact a completely zero header (w/ 1 byte of data... can also be zero) is still a valid CCSDS message (although as a "continuation segment" it's fairly unlikely).

So we certainly could just say "don't use 0", it's not invalid from a CCSDS point of view for the currently defined MsgID interpretations. I'm not against the idea of making 0 invalid, just need to be clear as to the relation to CCSDS (none of our current implementations would actually make 0 MsgID "invalid" from a CCSDS perspective).

[skliper]

It certainly does make clearing the message map and routing tables easier... But clearing a CCSDS primary header doesn't really make it "invalid" from a CCSDS perspective. Not that we can't say it's invalid for use with cFS. If only they would have used Packet Version Number 1...

[skliper]

We may run into some interoperability issues, since some designs may implement a "broadcast" sort of concept, for which 0 is a common choice. Not w/ cFS systems, but possibly others. They'd likely not have the same MsgID concept, but the header bits that correspond to their concept of a broadcast may line up with whatever MsgID implementation is used in cFS. Just a risk of arbitrarily saying a certain value in a header is "invalid".

[jphickey]

Yeah I am porting/merging code across multiple different branches, and turns out I was looking at a branch where MsgId was still defined as 16 bits.

With a 32-bit MsgId this is much easier, just set some fixed bit(s) in the upper 16 to ensure that the value is always nonzero for real IDs. They can be simply masked out in the SBR table lookup.