Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject duplicate message ID in filter table validation #106

Open
3 tasks done
skliper opened this issue Apr 26, 2023 · 1 comment
Open
3 tasks done

Reject duplicate message ID in filter table validation #106

skliper opened this issue Apr 26, 2023 · 1 comment
Labels

Comments

@skliper
Copy link
Contributor

skliper commented Apr 26, 2023

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
DS doesn't reject duplicate message ID entry in the filter table. Although SB does report the duplicate:

66/1/CFE_SB 7: Duplicate Subscription,MsgId 0x901 on DS_CMD_PIPE pipe,app DS

When DS does the lookup it only returns the first hit:

DS/fsw/src/ds_table.c

Lines 1120 to 1135 in 6418164

/* NULL when list is empty or end of list */
while (HashLink != (DS_HashLink_t *)NULL)
{
/* Compare this linked list entry for matching MessageID */
if (CFE_SB_MsgIdToValue(FilterPackets[HashLink->Index].MessageID) == CFE_SB_MsgIdToValue(MessageID))
{
/* Stop the search - we found it */
FilterTableIndex = HashLink->Index;
break;
}
/* Max of 8 links per design */
HashLink = HashLink->Next;
}
return FilterTableIndex;

Describe the solution you'd like
Since duplicate message ID in filter table isn't supported, should clearly reject an attempt by the user to do so. I suggest failing filter table validation.

Describe alternatives you've considered
None

Additional context
Can send to multiple destinations from a single entry, and supports changing those destinations via command so no additional functionality needed there. Just should reject multiple filter table entries with the same MID (user error).

Requester Info
Jacob Hageman - NASA/GSFC

@skliper skliper changed the title DS duplicate message ID in filter table handling DS reject duplicate message ID in filter table validation Apr 26, 2023
@skliper skliper changed the title DS reject duplicate message ID in filter table validation Reject duplicate message ID in filter table validation Apr 26, 2023
@havencarlson havencarlson self-assigned this Sep 5, 2023
@havencarlson havencarlson removed their assignment Sep 28, 2023
@skliper skliper added the bug label Apr 26, 2024
@skliper
Copy link
Contributor Author

skliper commented Apr 26, 2024

Raised to a bug since DS really shouldn't accept an invalid filter table (duplicate message ids). Hit this again in real development, and since it's not uncommon for SB events to get squelched if there's no subscribers for any msgids at startup the duplicate event can get filtered, and even if it isn't it's easy to get lost in the event noise at startup. Locally I changed DS_TableSubscribe to:

        /* Only process valid message id's, invalid is OK in table as an empty entry */
        if (CFE_SB_IsValidMsgId(MessageID))
        {

            /* Check for unsupported multiple entries (broken filter table) */
            /* Note can't use DS_TableFindMsgID since hash table isn't built yet */
            for (j = 0; j < i; j++)
            {
                if (CFE_SB_MsgId_Equal(FilterPackets[i].MessageID, FilterPackets[j].MessageID))
                {
                    CFE_ES_WriteToSysLog("  ***** DS Filter table multiple entries for MID 0x%08X, i = %d, j = %d",
                                         (unsigned int)CFE_SB_MsgIdToValue(MessageID), i, j);
                }
            }

            /*
            ** Already subscribed to DS command packets...
            */
            if ((CFE_SB_MsgIdToValue(MessageID) != DS_CMD_MID) && (CFE_SB_MsgIdToValue(MessageID) != DS_SEND_HK_MID))
            {
                CFE_SB_SubscribeEx(MessageID, DS_AppData.CmdPipe, CFE_SB_DEFAULT_QOS, DS_PER_PACKET_PIPE_LIMIT);
            }
        }

but it should probably be in the table validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants