From f3f3b65cd9b645e9cde9c179109d121cdc100f5f Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 27 Apr 2020 17:09:20 -0400 Subject: [PATCH] Fix #437, add new API for obtaining exception task ID Deprecate the OStask_ID field within the property structure. Replace with a new API to perform a task ID reverse-lookup based on an abstract data object (the system task ID). --- src/os/inc/osapi-os-core.h | 27 ++++++++++- src/os/posix/src/os-impl-tasks.c | 39 +++++++++++++++- src/os/rtems/src/os-impl-tasks.c | 33 +++++++++++++ src/os/shared/inc/os-shared-task.h | 21 +++++++++ src/os/shared/src/osapi-task.c | 38 +++++++++++++++ src/os/vxworks/src/os-impl-tasks.c | 36 +++++++++++++++ .../shared/src/coveragetest-task.c | 46 +++++++++++++++++++ .../ut-stubs/src/osapi-task-impl-stubs.c | 7 +++ .../vxworks/src/coveragetest-tasks.c | 34 ++++++++++++++ src/ut-stubs/osapi-utstub-task.c | 1 - 10 files changed, 279 insertions(+), 3 deletions(-) diff --git a/src/os/inc/osapi-os-core.h b/src/os/inc/osapi-os-core.h index 928561756..a9c1f336d 100644 --- a/src/os/inc/osapi-os-core.h +++ b/src/os/inc/osapi-os-core.h @@ -73,7 +73,9 @@ typedef struct uint32 creator; uint32 stack_size; uint32 priority; - uint32 OStask_id; +#ifndef OSAL_OMIT_DEPRECATED + uint32 OStask_id; /**< @deprecated */ +#endif }OS_task_prop_t; /** @brief OSAL queue properties */ @@ -479,6 +481,29 @@ int32 OS_TaskGetIdByName (uint32 *task_id, const char *task_name); * @retval #OS_INVALID_POINTER if the task_prop pointer is NULL */ int32 OS_TaskGetInfo (uint32 task_id, OS_task_prop_t *task_prop); + + +/*-------------------------------------------------------------------------------------*/ +/** + * @brief Reverse-lookup the OSAL task ID from an operating system ID + * + * This provides a method by which an external entity may find the OSAL task + * ID corresponding to a system-defined identifier (e.g. TASK_ID, pthread_t, rtems_id, etc). + * + * Normally OSAL does not expose the underlying OS-specific values to the application, + * but in some circumstances, such as exception handling, the OS may provide this information + * directly to handler outside of the normal OSAL API. + * + * @param[out] task_id The buffer where the task id output is stored + * @param[in] sysdata Pointer to the system-provided identification data + * @param[in] sysdata_size Size of the system-provided identification data + * + * @return Execution status, see @ref OSReturnCodes + * @retval #OS_SUCCESS @copybrief OS_SUCCESS + */ +int32 OS_TaskFindIdBySystemData(uint32 *task_id, const void *sysdata, size_t sysdata_size); + + /**@}*/ /** @defgroup OSAPIMsgQueue OSAL Message Queue APIs diff --git a/src/os/posix/src/os-impl-tasks.c b/src/os/posix/src/os-impl-tasks.c index da781e1f1..19f88224a 100644 --- a/src/os/posix/src/os-impl-tasks.c +++ b/src/os/posix/src/os-impl-tasks.c @@ -761,7 +761,8 @@ uint32 OS_TaskGetId_Impl (void) *-----------------------------------------------------------------*/ int32 OS_TaskGetInfo_Impl (uint32 task_id, OS_task_prop_t *task_prop) { - size_t copy_sz; +#ifndef OSAL_OMIT_DEPRECATED + size_t copy_sz; /* * NOTE - this is not really valid, as you can't officially @@ -783,7 +784,43 @@ int32 OS_TaskGetInfo_Impl (uint32 task_id, OS_task_prop_t *task_prop) } memcpy(&task_prop->OStask_id, &OS_impl_task_table[task_id].id, copy_sz); +#endif return OS_SUCCESS; } /* end OS_TaskGetInfo_Impl */ + +/*---------------------------------------------------------------- + * + * Function: OS_TaskIdMatchSystemData_Impl + * + * Purpose: Implemented per internal OSAL API + * See prototype for argument/return detail + * + *-----------------------------------------------------------------*/ +bool OS_TaskIdMatchSystemData_Impl(void *ref, uint32 local_id, const OS_common_record_t *obj) +{ + const pthread_t *target = (const pthread_t *)ref; + + return (pthread_equal(*target, OS_impl_task_table[local_id].id) != 0); +} + +/*---------------------------------------------------------------- + * + * Function: OS_TaskValidateSystemData_Impl + * + * Purpose: Implemented per internal OSAL API + * See prototype for argument/return detail + * + *-----------------------------------------------------------------*/ +int32 OS_TaskValidateSystemData_Impl(const void *sysdata, uint32 sysdata_size) +{ + if (sysdata == NULL || sysdata_size != sizeof(pthread_t)) + { + return OS_INVALID_POINTER; + } + return OS_SUCCESS; +} + + + diff --git a/src/os/rtems/src/os-impl-tasks.c b/src/os/rtems/src/os-impl-tasks.c index d861f6764..361e0f75d 100644 --- a/src/os/rtems/src/os-impl-tasks.c +++ b/src/os/rtems/src/os-impl-tasks.c @@ -328,9 +328,42 @@ uint32 OS_TaskGetId_Impl (void) *-----------------------------------------------------------------*/ int32 OS_TaskGetInfo_Impl (uint32 task_id, OS_task_prop_t *task_prop) { +#ifndef OSAL_OMIT_DEPRECATED task_prop->OStask_id = (uint32) OS_impl_task_table[task_id].id; +#endif return OS_SUCCESS; } /* end OS_TaskGetInfo_Impl */ +/*---------------------------------------------------------------- + * + * Function: OS_TaskValidateSystemData_Impl + * + * Purpose: Implemented per internal OSAL API + * See prototype for argument/return detail + * + *-----------------------------------------------------------------*/ +int32 OS_TaskValidateSystemData_Impl(const void *sysdata, uint32 sysdata_size) +{ + if (sysdata == NULL || sysdata_size != sizeof(rtems_id)) + { + return OS_INVALID_POINTER; + } + return OS_SUCCESS; +} + +/*---------------------------------------------------------------- + * + * Function: OS_TaskIdMatchSystemData_Impl + * + * Purpose: Implemented per internal OSAL API + * See prototype for argument/return detail + * + *-----------------------------------------------------------------*/ +bool OS_TaskIdMatchSystemData_Impl(void *ref, uint32 local_id, const OS_common_record_t *obj) +{ + const rtems_id *target = (const rtems_id *)ref; + + return (*target == OS_impl_task_table[local_id].id); +} diff --git a/src/os/shared/inc/os-shared-task.h b/src/os/shared/inc/os-shared-task.h index 644e9a14e..226dd98cc 100644 --- a/src/os/shared/inc/os-shared-task.h +++ b/src/os/shared/inc/os-shared-task.h @@ -158,5 +158,26 @@ int32 OS_TaskGetInfo_Impl (uint32 task_id, OS_task_prop_t *task_prop) int32 OS_TaskRegister_Impl (uint32 global_task_id); +/*---------------------------------------------------------------- + + Function: OS_TaskIdMatchSystemData_Impl + + Purpose: A helper "match" function to find an OSAL task ID based on system ID + Compatible with the "OS_ObjectIdFindBySearch" routine + + ------------------------------------------------------------------*/ +bool OS_TaskIdMatchSystemData_Impl(void *ref, uint32 local_id, const OS_common_record_t *obj); + +/*---------------------------------------------------------------- + + Function: OS_TaskValidateSystemData_Impl + + Purpose: Checks that the supplied sysdata pointer and sysdata_size are + compatible/reasonable for the underlying OS. + + ------------------------------------------------------------------*/ +int32 OS_TaskValidateSystemData_Impl(const void *sysdata, uint32 sysdata_size); + + #endif /* INCLUDE_OS_SHARED_TASK_H_ */ diff --git a/src/os/shared/src/osapi-task.c b/src/os/shared/src/osapi-task.c index 4c925424d..d3946f9d1 100644 --- a/src/os/shared/src/osapi-task.c +++ b/src/os/shared/src/osapi-task.c @@ -512,3 +512,41 @@ int32 OS_TaskInstallDeleteHandler(osal_task_entry function_pointer) return return_code; } /* end OS_TaskInstallDeleteHandler */ + +/*---------------------------------------------------------------- + * + * Function: OS_TaskFindIdBySystemData + * + * Purpose: Implemented per public OSAL API + * See description in API and header file for detail + * + *-----------------------------------------------------------------*/ +int32 OS_TaskFindIdBySystemData(uint32 *task_id, const void *sysdata, size_t sysdata_size) +{ + int32 return_code; + OS_common_record_t *record; + + /* Check parameters */ + if (task_id == NULL) + { + return OS_INVALID_POINTER; + } + + /* The "sysdata" and "sysdata_size" must be passed to the underlying impl for validation */ + return_code = OS_TaskValidateSystemData_Impl(sysdata, sysdata_size); + if (return_code != OS_SUCCESS) + { + return return_code; + } + + return_code = OS_ObjectIdGetBySearch(OS_LOCK_MODE_GLOBAL, LOCAL_OBJID_TYPE, OS_TaskIdMatchSystemData_Impl, (void*)sysdata, &record); + if (return_code == OS_SUCCESS) + { + *task_id = record->active_id; + OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + } + + return return_code; +} /* end OS_TaskFindIdBySystemData */ + + diff --git a/src/os/vxworks/src/os-impl-tasks.c b/src/os/vxworks/src/os-impl-tasks.c index 08cb45881..b9c430eb7 100644 --- a/src/os/vxworks/src/os-impl-tasks.c +++ b/src/os/vxworks/src/os-impl-tasks.c @@ -420,6 +420,7 @@ uint32 OS_TaskGetId_Impl (void) *-----------------------------------------------------------------*/ int32 OS_TaskGetInfo_Impl (uint32 task_id, OS_task_prop_t *task_prop) { +#ifndef OSAL_OMIT_DEPRECATED union { TASK_ID vxid; @@ -434,8 +435,43 @@ int32 OS_TaskGetInfo_Impl (uint32 task_id, OS_task_prop_t *task_prop) */ u.vxid = OS_impl_task_table[task_id].vxid; task_prop->OStask_id = u.value; +#endif return OS_SUCCESS; } /* end OS_TaskGetInfo_Impl */ +/*---------------------------------------------------------------- + * + * Function: OS_TaskValidateSystemData_Impl + * + * Purpose: Implemented per internal OSAL API + * See prototype for argument/return detail + * + *-----------------------------------------------------------------*/ +int32 OS_TaskValidateSystemData_Impl(const void *sysdata, uint32 sysdata_size) +{ + if (sysdata == NULL || sysdata_size != sizeof(TASK_ID)) + { + return OS_INVALID_POINTER; + } + return OS_SUCCESS; +} + +/*---------------------------------------------------------------- + * + * Function: OS_TaskIdMatchSystemData_Impl + * + * Purpose: Implemented per internal OSAL API + * See prototype for argument/return detail + * + *-----------------------------------------------------------------*/ +bool OS_TaskIdMatchSystemData_Impl(void *ref, uint32 local_id, const OS_common_record_t *obj) +{ + const TASK_ID *target = (const TASK_ID *)ref; + + return (*target == OS_impl_task_table[local_id].vxid); +} + + + diff --git a/src/unit-test-coverage/shared/src/coveragetest-task.c b/src/unit-test-coverage/shared/src/coveragetest-task.c index 230571665..df31123db 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-task.c +++ b/src/unit-test-coverage/shared/src/coveragetest-task.c @@ -292,6 +292,51 @@ void Test_OS_TaskInstallDeleteHandler(void) OS_task_table[1].delete_hook_pointer = NULL; } +void Test_OS_TaskFindIdBySystemData(void) +{ + /* + * Test Case For: + * int32 OS_TaskFindIdBySystemData(uint32 *task_id, const void *sysdata, size_t sysdata_size) + */ + + int32 expected; + int32 actual; + uint32 task_id; + + /* + * Use a compound data struct for the system data. + * The intent is to intentionally make something bigger that will not fit into e.g. "uint32" + */ + struct + { + unsigned long v; + void *p; + } test_sysdata; + + memset(&test_sysdata, 'x', sizeof(test_sysdata)); + + expected = OS_SUCCESS; + actual = OS_TaskFindIdBySystemData(&task_id, &test_sysdata, sizeof(test_sysdata)); + UtAssert_True(actual == expected, "OS_TaskFindIdBySystemData() (%ld) == OS_SUCCESS", (long)actual); + + /* Test parameter validation branches */ + expected = OS_INVALID_POINTER; + actual = OS_TaskFindIdBySystemData(NULL, &test_sysdata, sizeof(test_sysdata)); + UtAssert_True(actual == expected, "OS_TaskFindIdBySystemData() (%ld) == OS_INVALID_POINTER", (long)actual); + + UT_SetForceFail(UT_KEY(OS_TaskValidateSystemData_Impl), expected); + actual = OS_TaskFindIdBySystemData(&task_id, &test_sysdata, sizeof(test_sysdata)); + UtAssert_True(actual == expected, "OS_TaskFindIdBySystemData() (%ld) == OS_INVALID_POINTER", (long)actual); + UT_ClearForceFail(UT_KEY(OS_TaskValidateSystemData_Impl)); + + /* Test search failure */ + expected = OS_ERR_NAME_NOT_FOUND; + UT_SetForceFail(UT_KEY(OS_ObjectIdGetBySearch), expected); + actual = OS_TaskFindIdBySystemData(&task_id, &test_sysdata, sizeof(test_sysdata)); + UtAssert_True(actual == expected, "OS_TaskFindIdBySystemData() (%ld) == OS_ERR_NAME_NOT_FOUND", (long)actual); + UT_ClearForceFail(UT_KEY(OS_ObjectIdGetBySearch)); +} + /* Osapi_Test_Setup * @@ -332,6 +377,7 @@ void UtTest_Setup(void) ADD_TEST(OS_TaskGetIdByName); ADD_TEST(OS_TaskGetInfo); ADD_TEST(OS_TaskInstallDeleteHandler); + ADD_TEST(OS_TaskFindIdBySystemData); } diff --git a/src/unit-test-coverage/ut-stubs/src/osapi-task-impl-stubs.c b/src/unit-test-coverage/ut-stubs/src/osapi-task-impl-stubs.c index e4abe6c48..9cf4cbdfa 100644 --- a/src/unit-test-coverage/ut-stubs/src/osapi-task-impl-stubs.c +++ b/src/unit-test-coverage/ut-stubs/src/osapi-task-impl-stubs.c @@ -48,3 +48,10 @@ uint32 OS_TaskGetId_Impl (void) UT_DEFAULT_STUB(OS_TaskGetInfo_Impl,(uint32 task_id, OS_task_prop_t *task_prop)) UT_DEFAULT_STUB(OS_TaskRegister_Impl,(uint32 global_task_id)) +bool OS_TaskIdMatchSystemData_Impl(void *ref, uint32 local_id, const OS_common_record_t *obj) +{ + return UT_DEFAULT_IMPL(OS_TaskIdMatchSystemData_Impl); +} + +UT_DEFAULT_STUB(OS_TaskValidateSystemData_Impl,(const void *sysdata, uint32 sysdata_size)) + diff --git a/src/unit-test-coverage/vxworks/src/coveragetest-tasks.c b/src/unit-test-coverage/vxworks/src/coveragetest-tasks.c index cc9f5752f..5fe2162a6 100644 --- a/src/unit-test-coverage/vxworks/src/coveragetest-tasks.c +++ b/src/unit-test-coverage/vxworks/src/coveragetest-tasks.c @@ -188,6 +188,38 @@ void Test_OS_TaskGetInfo_Impl(void) OSAPI_TEST_FUNCTION_RC(OS_TaskGetInfo_Impl(0,&task_prop), OS_SUCCESS); } +void Test_OS_TaskValidateSystemData_Impl(void) +{ + /* + * Test Case For: + * int32 OS_TaskValidateSystemData_Impl(const void *sysdata, uint32 sysdata_size) + */ + OCS_TASK_ID test_sys_id; + + memset(&test_sys_id, 'x', sizeof(test_sys_id)); + + OSAPI_TEST_FUNCTION_RC(OS_TaskValidateSystemData_Impl(&test_sys_id, sizeof(test_sys_id)), OS_SUCCESS); + OSAPI_TEST_FUNCTION_RC(OS_TaskValidateSystemData_Impl(NULL, sizeof(test_sys_id)), OS_INVALID_POINTER); + OSAPI_TEST_FUNCTION_RC(OS_TaskValidateSystemData_Impl(&test_sys_id, sizeof(test_sys_id)-1), OS_INVALID_POINTER); +} + +void Test_OS_TaskIdMatchSystemData_Impl(void) +{ + /* + * Test Case For: + * bool OS_TaskIdMatchSystemData_Impl(void *ref, uint32 local_id, const OS_common_record_t *obj) + */ + OCS_TASK_ID test_sys_id; + + memset(&test_sys_id, 'x', sizeof(test_sys_id)); + + UT_TaskTest_SetImplTaskId(0, test_sys_id); + OSAPI_TEST_FUNCTION_RC(OS_TaskIdMatchSystemData_Impl(&test_sys_id, 0, NULL), true); + + memset(&test_sys_id, 'y', sizeof(test_sys_id)); + OSAPI_TEST_FUNCTION_RC(OS_TaskIdMatchSystemData_Impl(&test_sys_id, 0, NULL), false); +} + /* ------------------- End of test cases --------------------------------------*/ @@ -234,6 +266,8 @@ void UtTest_Setup(void) ADD_TEST(OS_TaskRegister_Impl); ADD_TEST(OS_TaskGetId_Impl); ADD_TEST(OS_TaskGetInfo_Impl); + ADD_TEST(OS_TaskValidateSystemData_Impl); + ADD_TEST(OS_TaskIdMatchSystemData_Impl); } diff --git a/src/ut-stubs/osapi-utstub-task.c b/src/ut-stubs/osapi-utstub-task.c index 8b6747748..bcaa5bfc7 100644 --- a/src/ut-stubs/osapi-utstub-task.c +++ b/src/ut-stubs/osapi-utstub-task.c @@ -264,7 +264,6 @@ int32 OS_TaskGetInfo(uint32 task_id, OS_task_prop_t *task_prop) { task_prop->creator = 1; UT_FIXUP_ID(task_prop->creator, UT_OBJTYPE_TASK); - task_prop->OStask_id = task_id & 0xFFFF; task_prop->stack_size = 100; task_prop->priority = 150; strncpy(task_prop->name, "UnitTest", OS_MAX_API_NAME - 1);