Fix #1076: compact ActiveSubs array in TO_LAB_RemovePacketCmd to prev…

[Adityakk9031]

Summary

Fixes an externally-triggerable state-machine desynchronization in TO_LAB where ActiveSubCount could diverge from the actual reusable occupancy of ActiveSubs[], causing valid subscription tracking loss and false capacity exhaustion.

Closes nasa/cFS#1076

Problem

TO_LAB_AddPacketCmd() and TO_LAB_RemovePacketCmd() maintained the dynamic subscription table inconsistently.

AddPacket (to_lab_cmds.c)

• Gated admission on ActiveSubCount >= TO_LAB_MISSION_MAX_SUBSCRIPTIONS
• Always wrote new entries to ActiveSubs[ActiveSubCount] (assumes array is contiguous)
• Incremented ActiveSubCount

RemovePacket (to_lab_cmds.c)

• Found the matched entry by scanning the array
• Zeroed that slot:

  ActiveSubs[ActiveSubsIndex] = CFE_SB_ValueToMsgId(0);

• Decremented ActiveSubCount
• Did not compact the array, leaving a hole at ActiveSubsIndex

Consequence

After one or more removals, holes existed at lower indices while AddPacket kept appending only at the tail (ActiveSubs[ActiveSubCount]), overwriting valid existing subscriptions that were still active. Eventually ActiveSubCount reached MAX while reusable cleared slots still existed, causing:

• Original subscriptions silently lost (overwritten, not removed from SB)
• New subscriptions falsely rejected with "max subscriptions reached"

Dynamically Reproduced Evidence (from issue report)

TO RemovePkt 0x800
TO RemovePkt 0x801
TO RemovePkt 0x803
TO AddPkt 0x9a0
TO AddPkt 0x9a1
...
TO Lab Rm Pkt Error: Stream 0x8fb, missing from active subscriptions
...
TO Can't subscribe to 0x9e0, max subscriptions (50) reached

The removed packets create low-index holes. Subsequent additions continue writing at the tail, eventually overwriting active subscriptions. Later, valid subscriptions appear missing even though they were never explicitly removed, and new subscriptions are rejected despite available reusable slots.

Fix

In TO_LAB_RemovePacketCmd(), after a successful CFE_SB_Unsubscribe, compact the array by moving the last active entry into the freed slot, then clearing the old tail.

-    /* clear the active subscription entry */
-    TO_LAB_Global.ActiveSubs[ActiveSubsIndex] = CFE_SB_ValueToMsgId(0);
+    /* Compact array: move the last active subscription into the freed slot */
+    TO_LAB_Global.ActiveSubs[ActiveSubsIndex]                  =
+        TO_LAB_Global.ActiveSubs[TO_LAB_Global.ActiveSubCount - 1];
+    TO_LAB_Global.ActiveSubs[TO_LAB_Global.ActiveSubCount - 1] =
+        CFE_SB_ValueToMsgId(0);
     --TO_LAB_Global.ActiveSubCount;

This maintains the invariant:

• ActiveSubs[0 .. ActiveSubCount - 1] is always contiguous and fully occupied.
• ActiveSubs[ActiveSubCount ..] is always zero/invalid.

Since AddPacket writes to ActiveSubs[ActiveSubCount], it is now guaranteed to always write to a genuinely empty slot without colliding with any active subscription.

Verification

Scenario	Before Fix	After Fix
Remove low-index entry, then add new entry	New entry overwrites valid active subscription	New entry placed at correct empty tail slot
Original subscription not explicitly removed	Reported missing after overwrite	Always preserved
Add after holes exist	Rejected as max reached despite free slots	Capacity gate reflects true occupancy
Remove last active entry (self-copy case)	N/A	ActiveSubs[i] = ActiveSubs[i] is a benign no-op
TO_LAB_RemoveAllCmd interaction	Independent full-array sweep	Unaffected
Duplicate check in AddPacketCmd	N/A	Cleared slots contain CFE_SB_ValueToMsgId(0), so they never match a valid stream

Related

• Issue: [SECURITY] TO_LAB Remove command desynchronizes ActiveSubCount from actual subscription slot usage causing false capacity exhaustion cFS#1076
• Modified file: fsw/src/to_lab_cmds.c
• Function: TO_LAB_RemovePacketCmd()

[Adityakk9031]

@elizabash and @jphickey have a look