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

OSAL path length variables cause TBL tests to fail #2372

Closed
irowebbn opened this issue Jun 8, 2023 · 3 comments · Fixed by #2373
Closed

OSAL path length variables cause TBL tests to fail #2372

irowebbn opened this issue Jun 8, 2023 · 3 comments · Fixed by #2373

Comments

@irowebbn
Copy link
Contributor

irowebbn commented Jun 8, 2023

Describe the bug
A clear and concise description of what the bug is.

For certain values of OSAL_CONFIG_MAX_FILE_NAME and OSAL_CONFIG_MAX_PATH_LEN, tests for TBL (and the DS application) fail. I suspect that the tests are making assumptions about the maximum values these can be set to.

To Reproduce
Steps to reproduce the behavior:

  1. Build FSW with OSAL_CONFIG_MAX_FILE_NAME set to 64 and OSAL_CONFIG_MAX_PATH_LEN set to 128
  2. Test with
ctest coverage -VV --test-dir build/intel64/default_intel64/ -R coverage-tbl-ALL
ctest coverage -VV --test-dir build/intel64/default_intel64/ -R coverage-ds-ds_file

Expected behavior
A clear and concise description of what you expected to happen.
Expect tests to pass.

Code snips
If applicable, add references to the software.

1: [BEGIN] 33 Test_CFE_TBL_TblMod
81: [ INFO] tbl_UT.c:3040:Begin Test Table Modified
... < many lines >
81: [ FAIL] 33.022 tbl_UT.c:3179 - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa == aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, 124
81: [ FAIL] 33.023 tbl_UT.c:3181 - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa == (*)
81: [ PASS] 33.024 tbl_UT.c:3184 - CFE_TBL_Modified(CFE_TBL_BAD_TABLE_HANDLE) (-872415231) == CFE_TBL_ERR_INVALID_HANDLE (-872415231)
81: [  END] 33 Test_CFE_TBL_TblMod  TOTAL::24    PASS::22    FAIL::2     MIR::0     TSF::0     TTF::0     WARN::0   

System observed on:

  • Hardware: Intel(R) Xeon(R) CPU E5-2687W v4 @ 3.00GHz
  • OS: CentOS 7 Linux
  • Versions: draco-rc3 tags for cFE, OSAL, and apps

Additional context
Add any other context about the problem here.
related issue in DS: nasa/DS#111

Reporter Info
Full name and company/organization if applicable
Isaac Rowe, NASA JSC/Jacobs Technology

@irowebbn
Copy link
Contributor Author

irowebbn commented Jun 8, 2023

There's no documented restriction in OSAL on the value of these variables.

@thnkslprpt
Copy link
Contributor

Updating the matching CFE_MISSION_MAX_FILE_LEN (to 64) and CFE_MISSION_MAX_PATH_LEN (to 128) will stop the test failures in cFE.

Those tests in TBL end up referencing both the OSAL_CONFIG parameters and the CFE_MISSION config parameters (through the CFE_TBL_Info_t struct).

@irowebbn
Copy link
Contributor Author

irowebbn commented Jun 9, 2023

Thanks for the info. I can certainly update the test to only test up to the truncated length. However, it may be worth some thinking deeper about the implications of this design-- for example, since the filename is truncated when fetched via CFE_TBL_GetInfo, you may lose the "(*)" that is added to the end of the filename when the table is modified. Maybe we should have a compile-time check to enforce that the path sizes specified by the CFE mission config are >= the largest path size specified by the OS configs across all flight processors?

irowebbn added a commit to irowebbn/cFE that referenced this issue Jun 15, 2023
irowebbn added a commit to irowebbn/cFE that referenced this issue Nov 30, 2023
dzbaker added a commit that referenced this issue Dec 5, 2023
Fix #2372 TBL UT update for OSAL/CFE path length mismatch
dzbaker added a commit that referenced this issue Dec 5, 2023
Fix #2372 TBL UT update for OSAL/CFE path length mismatch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants