Skip to content

Fix #148, Add table access protections - #150

Merged
jphickey merged 1 commit into
nasa:devfrom
jphickey:fix-148-table-access
Jul 28, 2026
Merged

Fix #148, Add table access protections#150
jphickey merged 1 commit into
nasa:devfrom
jphickey:fix-148-table-access

Conversation

@jphickey

Copy link
Copy Markdown
Contributor

Checklist (Please check before submitting)

Describe the contribution
Scrub use of all table pointers and add access routines to get an entry from the table instead of dereferencing the array directly. This pattern allows easier verification that all access includes a NULL check.

Also includes some refactoring to support this goal. Notably this includes changing the bitmask used to track if an appmon entry is active or inactive.

Fixes #148

Testing performed
Send command sequence that caused failure
Updated unit tests to cover those cases

Expected behavior changes
No segfaults when accessing table data

System(s) tested on
Linux

Additional context
Strongly recommend follow up to continue consolidating code. This PR touches on some things related to #5 but there is a lot more to do, a lot of code is still repetitive and could be refactored. Also, many functions rank high in cyclic complexity. This reduces it slightly by breaking up some functions (e.g. HS_MonitorApplications went from 128 to 90 by breaking it up) but 90 is still too high. There is much more that should be done to make smaller functions.

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

Scrub use of all table pointers and add access routines to get an entry
from the table instead of dereferencing the array directly.  This pattern
allows easier verification that all access includes a NULL check.

Also includes some refactoring to support this goal.  Notably this
includes changing the bitmask used to track if an appmon entry is active
or inactive.
@jphickey
jphickey requested a review from ddstewar July 28, 2026 14:06
@ddstewar

Copy link
Copy Markdown

Tested on VM, with the table removed as stated in the issue this corrects. the following event were seen in response to the HS_CMD_ENABLE_APP_MON command:

EVS Port1 1980-012-14:07:01.46084 66/1/HS 25: Application Monitoring Enabled
EVS Port1 1980-012-14:07:02.66392 66/1/HS 33: Error getting AppMon Table address, RC=0xCC000005, Application Monitoring Disabled

The system did not crash and the table was reverted to Disabled once the background task recognized that it could not be accessed.

Approved

@jphickey
jphickey merged commit 828855f into nasa:dev Jul 28, 2026
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Null pointer dereference due to incomplete initialization.

3 participants