From 61d76af69474ec2cb2fc652fc9dde0f32e35695c Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Tue, 8 Nov 2022 10:55:42 -0500 Subject: [PATCH 1/6] Fix #1328, always provide UT stub library The "ut_osapi_stubs" library is mostly relevant for testing other applications, as opposed to testing OSAL itself. Therefore this library should be provided regardless of whether OSAL is being tested. --- CMakeLists.txt | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4bf4d875e..38a39952f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -49,13 +49,13 @@ # Normally this setting is not needed to be configured as it is # inferred from the BSP type. # -# OSAL_INSTALL_LIBRARIES : Boolean, enables "install" of targets listed above such +# OSAL_INSTALL_LIBRARIES : Boolean, enables "install" of targets listed above such # that libraries and public API header files are copied into the system # location specified by CMAKE_INSTALL_PREFIX. This set of headers # and link libraries can then be used to compile other applications # separately from OSAL. Default is "ON" when OSAL is being compiled # standalone (i.e. cmake called directly on the OSAL source dir). -# Default is "OFF" when OSAL is included via 'add_subdirectory' in +# Default is "OFF" when OSAL is included via 'add_subdirectory' in # a parent application project such as CFE/CFS. # # ENABLE_UNIT_TESTS : Boolean, enables build of the unit tests (coverage and functional) @@ -203,13 +203,19 @@ add_definitions(${OSAL_USER_C_FLAGS}) # # Build UT assert - # the basic ut_assert library target is always defined regardless of "ENABLE_UNIT_TESTS", -# but flagged using "EXCLUDE_FROM_ALL" so they won't be built unless actually used. This -# is because the library is usable with functional tests, not just unit (coverage) tests. +# This is because the library is usable with functional tests, not just unit (coverage) tests. # This is done early, so that other targets may reference UT_ASSERT_SOURCE_DIR if needed # Note that UT assert is only usable with an OSAL BSP implementation, thus this target # cannot be compiled unless a specific BSP is selected. add_subdirectory(ut_assert) +# The "ut_osapi_stubs" library contains stub functions of the OSAL API calls, used for +# testing _other_ application code that uses OSAL. Like UT Assert, this is also +# provided independently of the local ENABLE_UNIT_TESTS switch, because it mainly affects +# testing of other components. That is, even if OSAL itself is not being tested in this +# build, a dependent application may still be built with testing enabled. +add_subdirectory(src/ut-stubs ut-stubs) + # # Step 1: # Build the BSP layer @@ -410,10 +416,6 @@ if (ENABLE_UNIT_TESTS) endfunction(add_osal_ut_exe) - # The "ut_osapi_stubs" library contains "stub" functions of the OSAL API calls, used for - # testing other application code built on top of OSAL. - add_subdirectory(src/ut-stubs ut-stubs) - # The "unit-test-coverage" is a white-box line coverage test. # It re-compiles each source unit for coverage analysis and links # with stub dependencies and a test sequence designed to execute From 2ae7d7de4eb72cef0e55cba546102c782617aa70 Mon Sep 17 00:00:00 2001 From: dmknutsen Date: Tue, 15 Nov 2022 19:11:07 -0500 Subject: [PATCH 2/6] Fix #1270: Truncate symbol name if > OS_MAX_SYM_LEN --- src/os/vxworks/src/os-impl-symtab.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/os/vxworks/src/os-impl-symtab.c b/src/os/vxworks/src/os-impl-symtab.c index c3018fd48..a3699f6ca 100644 --- a/src/os/vxworks/src/os-impl-symtab.c +++ b/src/os/vxworks/src/os-impl-symtab.c @@ -161,11 +161,20 @@ BOOL OS_SymTableIterator_Impl(char *name, SYM_VALUE val, SYM_TYPE type, _Vx_usr_ */ state = &OS_VxWorks_SymbolDumpState; + /* + ** Copy symbol name + */ + strncpy(symRecord.SymbolName, name, sizeof(symRecord.SymbolName) - 1); + symRecord.SymbolName[sizeof(symRecord.SymbolName) - 1] = '\0'; + + /* + ** Check to see if the max length of each symbol name has been reached + */ if (memchr(name, 0, OS_MAX_SYM_LEN) == NULL) { + symRecord.SymbolName[sizeof(symRecord.SymbolName) - 2] = '*'; OS_DEBUG("%s(): symbol name too long\n", __func__); state->StatusCode = OS_ERR_NAME_TOO_LONG; - return false; } /* @@ -183,12 +192,6 @@ BOOL OS_SymTableIterator_Impl(char *name, SYM_VALUE val, SYM_TYPE type, _Vx_usr_ return false; } - /* - ** Copy symbol name - */ - strncpy(symRecord.SymbolName, name, sizeof(symRecord.SymbolName) - 1); - symRecord.SymbolName[sizeof(symRecord.SymbolName) - 1] = 0; - /* ** Save symbol address */ From ec4be238127bc9fb67b444cfb56dac29b051d507 Mon Sep 17 00:00:00 2001 From: dmknutsen Date: Tue, 15 Nov 2022 20:33:06 -0500 Subject: [PATCH 3/6] Fix #1270: Unit Test Update --- src/unit-test-coverage/vxworks/src/coveragetest-symtab.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c b/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c index 1df926e9b..b181c9147 100644 --- a/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c +++ b/src/unit-test-coverage/vxworks/src/coveragetest-symtab.c @@ -77,7 +77,7 @@ void Test_OS_SymTableIterator_Impl(void) /* Check case where entry has a name that is too long */ UT_SetDefaultReturnValue(UT_KEY(OCS_memchr), OS_ERROR); - OSAPI_TEST_FUNCTION_RC(UT_SymTabTest_CallIteratorFunc("ut", &Data, 100, 1000), false); + OSAPI_TEST_FUNCTION_RC(UT_SymTabTest_CallIteratorFunc("ut", &Data, 100, 1000), true); OSAPI_TEST_FUNCTION_RC(UT_SymTabTest_GetIteratorStatus(), OS_ERR_NAME_TOO_LONG); UT_ClearDefaultReturnValue(UT_KEY(OCS_memchr)); From 244cf7d4f946dc861a9d7380097407a2ec119b05 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 17 Nov 2022 15:43:59 -0500 Subject: [PATCH 4/6] Fix #1324, use cmake config for cppcheck Passes a cmake configuration so cppcheck can use the exported commands, allowing it to use all header files that the actual build uses. --- .github/workflows/static-analysis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index ac905d695..ea2250b04 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -4,10 +4,12 @@ name: Static Analysis on: push: pull_request: + workflow_dispatch: jobs: static-analysis: - name: Run cppcheck + name: Run Static Analysis uses: nasa/cFS/.github/workflows/static-analysis.yml@main with: strict-dir-list: './src/bsp ./src/os' + cmake-project-options: -DENABLE_UNIT_TESTS=TRUE -DOSAL_OMIT_DEPRECATED=TRUE -DOSAL_SYSTEM_BSPTYPE=generic-linux From ff110cce36b34cc92446ecf79469c50c94a23113 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 17 Nov 2022 15:42:31 -0500 Subject: [PATCH 5/6] Fix #1325, resolve cppcheck warnings Resolves a number of issues reported by cppcheck. --- src/os/portable/os-impl-posix-dl-symtab.c | 2 +- src/os/posix/src/os-impl-console.c | 2 ++ src/os/posix/src/os-impl-tasks.c | 14 ++++++++++---- src/os/posix/src/os-impl-timebase.c | 10 +++++++--- src/os/rtems/src/os-impl-filesys.c | 10 ++++++---- src/os/rtems/src/os-impl-timebase.c | 6 ++++-- .../oscore-test/ut_oscore_countsem_test.c | 18 ++++++++++-------- 7 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/os/portable/os-impl-posix-dl-symtab.c b/src/os/portable/os-impl-posix-dl-symtab.c index 174e4e157..1a67ec19f 100644 --- a/src/os/portable/os-impl-posix-dl-symtab.c +++ b/src/os/portable/os-impl-posix-dl-symtab.c @@ -137,7 +137,7 @@ int32 OS_GenericSymbolLookup_Impl(void *dl_handle, cpuaddr *SymbolAddress, const int32 OS_SymbolLookup_Impl(cpuaddr *SymbolAddress, const char *SymbolName) { int32 status; - int32 local_status = OS_ERROR; + int32 local_status; OS_object_iter_t iter; /* First search global table */ diff --git a/src/os/posix/src/os-impl-console.c b/src/os/posix/src/os-impl-console.c index 1795a9cac..9d735ff6d 100644 --- a/src/os/posix/src/os-impl-console.c +++ b/src/os/posix/src/os-impl-console.c @@ -80,6 +80,7 @@ static void *OS_ConsoleTask_Entry(void *arg) OS_impl_console_internal_record_t *local; OS_object_token_t token; + /* cppcheck-suppress unreadVariable // intentional use of other union member */ local_arg.opaque_arg = arg; if (OS_ObjectIdGetById(OS_LOCK_MODE_REFCOUNT, OS_OBJECT_TYPE_OS_CONSOLE, local_arg.id, &token) == OS_SUCCESS) { @@ -125,6 +126,7 @@ int32 OS_ConsoleCreate_Impl(const OS_object_token_t *token) } else { + /* cppcheck-suppress unreadVariable // intentional use of other union member */ local_arg.id = OS_ObjectIdFromToken(token); return_code = OS_Posix_InternalTaskCreate_Impl(&consoletask, OS_CONSOLE_TASK_PRIORITY, 0, OS_ConsoleTask_Entry, local_arg.opaque_arg); diff --git a/src/os/posix/src/os-impl-tasks.c b/src/os/posix/src/os-impl-tasks.c index a90a8c9c7..f41428661 100644 --- a/src/os/posix/src/os-impl-tasks.c +++ b/src/os/posix/src/os-impl-tasks.c @@ -114,6 +114,7 @@ static void *OS_PthreadTaskEntry(void *arg) { OS_VoidPtrValueWrapper_t local_arg; + /* cppcheck-suppress unreadVariable // intentional use of other union member */ local_arg.opaque_arg = arg; OS_TaskEntryPoint(local_arg.id); /* Never returns */ @@ -574,8 +575,10 @@ int32 OS_TaskCreate_Impl(const OS_object_token_t *token, uint32 flags) OS_impl_task_internal_record_t *impl; OS_task_internal_record_t * task; - arg.opaque_arg = NULL; - arg.id = OS_ObjectIdFromToken(token); + memset(&arg, 0, sizeof(arg)); + + /* cppcheck-suppress unreadVariable // intentional use of other union member */ + arg.id = OS_ObjectIdFromToken(token); task = OS_OBJECT_TABLE_GET(OS_task_table, *token); impl = OS_OBJECT_TABLE_GET(OS_impl_task_table, *token); @@ -787,8 +790,10 @@ int32 OS_TaskRegister_Impl(osal_id_t global_task_id) pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_state); pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, &old_type); - arg.opaque_arg = 0; - arg.id = global_task_id; + memset(&arg, 0, sizeof(arg)); + + /* cppcheck-suppress unreadVariable // intentional use of other union member */ + arg.id = global_task_id; return_code = pthread_setspecific(POSIX_GlobalVars.ThreadKey, arg.opaque_arg); if (return_code == 0) @@ -814,6 +819,7 @@ osal_id_t OS_TaskGetId_Impl(void) { OS_VoidPtrValueWrapper_t self_record; + /* cppcheck-suppress unreadVariable // intentional use of other union member */ self_record.opaque_arg = pthread_getspecific(POSIX_GlobalVars.ThreadKey); return self_record.id; diff --git a/src/os/posix/src/os-impl-timebase.c b/src/os/posix/src/os-impl-timebase.c index 6b4da6bd2..0edfd34d0 100644 --- a/src/os/posix/src/os-impl-timebase.c +++ b/src/os/posix/src/os-impl-timebase.c @@ -305,8 +305,10 @@ static void *OS_TimeBasePthreadEntry(void *arg) { OS_VoidPtrValueWrapper_t local_arg; + /* cppcheck-suppress unreadVariable // intentional use of other union member */ local_arg.opaque_arg = arg; OS_TimeBase_CallbackThread(local_arg.id); + return NULL; } @@ -341,9 +343,11 @@ int32 OS_TimeBaseCreate_Impl(const OS_object_token_t *token) * Note the thread will not actually start running until this function exits and releases * the global table lock. */ - arg.opaque_arg = NULL; - arg.id = OS_ObjectIdFromToken(token); - return_code = OS_Posix_InternalTaskCreate_Impl(&local->handler_thread, OSAL_PRIORITY_C(0), 0, + memset(&arg, 0, sizeof(arg)); + + /* cppcheck-suppress unreadVariable // intentional use of other union member */ + arg.id = OS_ObjectIdFromToken(token); + return_code = OS_Posix_InternalTaskCreate_Impl(&local->handler_thread, OSAL_PRIORITY_C(0), 0, OS_TimeBasePthreadEntry, arg.opaque_arg); if (return_code != OS_SUCCESS) { diff --git a/src/os/rtems/src/os-impl-filesys.c b/src/os/rtems/src/os-impl-filesys.c index a7f484fd5..177266a17 100644 --- a/src/os/rtems/src/os-impl-filesys.c +++ b/src/os/rtems/src/os-impl-filesys.c @@ -168,11 +168,13 @@ int32 OS_FileSysStartVolume_Impl(const OS_object_token_t *token) OS_DEBUG("rtems_blkdev_create() failed: %s.\n", rtems_status_text(sc)); return_code = OS_ERROR; } + else + { + OS_DEBUG("RAM disk initialized: volume=%s device=%s address=0x%08lX\n", local->volume_name, + impl->blockdev_name, (unsigned long)local->address); - OS_DEBUG("RAM disk initialized: volume=%s device=%s address=0x%08lX\n", local->volume_name, - impl->blockdev_name, (unsigned long)local->address); - - return_code = OS_SUCCESS; + return_code = OS_SUCCESS; + } break; } default: diff --git a/src/os/rtems/src/os-impl-timebase.c b/src/os/rtems/src/os-impl-timebase.c index 531ccd87d..b275d7cc5 100644 --- a/src/os/rtems/src/os-impl-timebase.c +++ b/src/os/rtems/src/os-impl-timebase.c @@ -471,8 +471,10 @@ int32 OS_TimeBaseSet_Impl(const OS_object_token_t *token, uint32 start_time, uin */ OS_UsecsToTicks(start_time, &start_ticks); - user_data.opaque_arg = NULL; - user_data.id = OS_ObjectIdFromToken(token); + memset(&user_data, 0, sizeof(user_data)); + + /* cppcheck-suppress unreadVariable // intentional use of other union member */ + user_data.id = OS_ObjectIdFromToken(token); status = rtems_timer_fire_after(local->rtems_timer_id, start_ticks, OS_TimeBase_ISR, user_data.opaque_arg); if (status != RTEMS_SUCCESSFUL) diff --git a/src/unit-tests/oscore-test/ut_oscore_countsem_test.c b/src/unit-tests/oscore-test/ut_oscore_countsem_test.c index 70138304c..786c13aff 100644 --- a/src/unit-tests/oscore-test/ut_oscore_countsem_test.c +++ b/src/unit-tests/oscore-test/ut_oscore_countsem_test.c @@ -93,16 +93,18 @@ void UT_os_count_sem_create_test() /* #4 Initial-count-too-high */ /* - * This test can only be done if the OS defines a specific "SEM_VALUE_MAX" - * The OSAL should define this for itself, but it currently does not. - * (This macro is not currently defined in RTEMS) + * The intent with this test case is to call OS_CountSemCreate() with an initial + * value greater than SEM_VALUE_MAX and confirm it returns OS_INVALID_SEM_VALUE. + * + * However, none of the currently available test platforms are able to produce + * this condition, because SEM_VALUE_MAX is either not defined/exposed or it + * is equal to UINT32_MAX and thus impossible to pass a value greater than this. + * + * Therefore a placeholder is here in case a platform in the future does permit + * it to be tested. Note that the check and return value is still tested in the + * coverage test for this function. */ -#if defined(SEM_VALUE_MAX) && SEM_VALUE_MAX < UINT32_MAX - UT_RETVAL(OS_CountSemCreate(&count_sem_ids[0], "CountSem1", ((uint32)SEM_VALUE_MAX) + 1, 0), OS_INVALID_SEM_VALUE, - "#4 Initial-count-too-high"); -#else UtAssert_NA("#4 Initial-count-too-high"); -#endif /*-----------------------------------------------------*/ /* #5 No-free-IDs */ From 69526c9c3828f55a49ac53e1e162d1943b156291 Mon Sep 17 00:00:00 2001 From: Dylan Date: Thu, 17 Nov 2022 16:10:40 -0500 Subject: [PATCH 6/6] Bump to v6.0.0-rc4+dev161 --- CHANGELOG.md | 6 ++++++ src/os/inc/osapi-version.h | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3990f5e35..e77e9b351 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Development Build: v6.0.0-rc4+dev161 +- Truncate symbol name if > OS_MAX_SYM_LEN +- always provide UT stub library +- cppcheck updates +- See , , and + ## Development Build: v6.0.0-rc4+dev151 - add BUGCHECK_VOID macro - See diff --git a/src/os/inc/osapi-version.h b/src/os/inc/osapi-version.h index b7ad6a2ae..ba46539da 100644 --- a/src/os/inc/osapi-version.h +++ b/src/os/inc/osapi-version.h @@ -34,7 +34,7 @@ /* * Development Build Macro Definitions */ -#define OS_BUILD_NUMBER 151 +#define OS_BUILD_NUMBER 161 #define OS_BUILD_BASELINE "v6.0.0-rc4" /*