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

Fix #1528, Move NO_SUCH_TABLE_ERR_EID into FindTableInRegistry and make optional #2274

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Mar 30, 2023

Checklist

Describe the contribution

  • Fixes Add option to send CFE_TBL_NO_SUCH_TABLE_ERR_EID event from CFE_TBL_FindTableInRegistry #1528
    • Removes several event reports of CFE_TBL_NO_SUCH_TABLE_ERR_EID from cfe_tbl_task_cmds.c where calls to CFE_TBL_FindTableInRegistry() return CFE_TBL_NOT_FOUND. (reducing code duplication)
    • Moves the event report into CFE_TBL_FindTableInRegistry() itself
    • Makes the event report optional by adding a new macro definition CFE_PLATFORM_TBL_SEND_EVENT_IF_TABLE_NOT_FOUND to switch the event report on (true) or off (false)

Testing performed
GitHub CI actions all passing successfully.
Local tests with cFS suite confirm no net loss of coverage.

Note: quite a few tests had to have an event count incremented up one because 3 functions in cfe_tbl_api.c were calling CFE_TBL_FindTableInRegistry() but were not sending an event on failure (like those from cfe_tbl_task_cmds.c were). So any unit tests calling these functions (including downstream) with intentional failures to find the table now issue an event, where they did not do so previously.

Expected behavior changes
Event reports of CFE_TBL_NO_SUCH_TABLE_ERR_EID are now all issued from within CFE_TBL_FindTableInRegistry() and can be optionally switched on or off from within the platform config file.

System(s) tested on
Intel(R) Celeron(R) N4100 CPU @ 1.10GHz x86_64
Debian GNU/Linux 11 (bullseye)
Current main branch of cFS.

Contributor Info
Avi Weiss @thnkslprpt

@github-advanced-security
Copy link

You have successfully added a new CodeQL configuration coding-standard. As part of the setup process, we have scanned this repository and found 1906 existing alerts. Please check the repository Security tab to see all alerts.

@@ -503,6 +503,14 @@
}
} while ((RegIndx == CFE_TBL_NOT_FOUND) && (i < (CFE_PLATFORM_TBL_MAX_NUM_TABLES - 1)));

#if (CFE_PLATFORM_TBL_SEND_EVENT_IF_TABLE_NOT_FOUND == true)

Check notice

Code scanning / CodeQL

Conditional compilation

Use of conditional compilation must be kept to a minimum.
@github-advanced-security
Copy link

You have successfully added a new CodeQL configuration security. As part of the setup process, we have scanned this repository and found 29 existing alerts. Please check the repository Security tab to see all alerts.

@thnkslprpt thnkslprpt force-pushed the fix-1528-move-tbl-not-found-eid-into-FindTableInRegistry branch from ae95ec6 to da5663a Compare May 5, 2023 04:21
@dzbaker dzbaker requested a review from chillfig June 8, 2023 18:31
Copy link
Contributor

@chillfig chillfig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes reduce duplication and enables optionality for the table check error as requested in the original issue. Thanks!

@thnkslprpt
Copy link
Contributor Author

No longer relevent after #2539

@thnkslprpt thnkslprpt closed this Apr 21, 2024
@thnkslprpt thnkslprpt deleted the fix-1528-move-tbl-not-found-eid-into-FindTableInRegistry branch April 21, 2024 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to send CFE_TBL_NO_SUCH_TABLE_ERR_EID event from CFE_TBL_FindTableInRegistry
2 participants