From 9c30ba51ee5cfc7b78330db02343d9d0afcdbd26 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 13 Aug 2020 13:24:20 -0400 Subject: [PATCH 1/4] Fix #557, remove OS_ObjectIdMap/Unmap prototype and stub These internal functions were no longer used or defined but the prototype and stub were still present. --- src/os/shared/inc/os-shared-idmap.h | 18 ------------ src/ut-stubs/osapi-utstub-idmap.c | 43 ----------------------------- 2 files changed, 61 deletions(-) diff --git a/src/os/shared/inc/os-shared-idmap.h b/src/os/shared/inc/os-shared-idmap.h index 2be4d37ec..5d61497e4 100644 --- a/src/os/shared/inc/os-shared-idmap.h +++ b/src/os/shared/inc/os-shared-idmap.h @@ -205,24 +205,6 @@ uint32 OS_GetMaxForObjectType(uint32 idtype); ------------------------------------------------------------------*/ uint32 OS_GetBaseForObjectType(uint32 idtype); -/*---------------------------------------------------------------- - Function: OS_ObjectIdMap - - Purpose: Convert an object serial number into a 32-bit OSAL ID of the given type - - Returns: OS_SUCCESS on success, or relevant error code - ------------------------------------------------------------------*/ -int32 OS_ObjectIdMap(uint32 idtype, uint32 idvalue, uint32 *result); - -/*---------------------------------------------------------------- - Function: OS_ObjectIdUnMap - - Purpose: Convert a 32-bit OSAL ID of the expected type into an object serial number - - Returns: OS_SUCCESS on success, or relevant error code - ------------------------------------------------------------------*/ -int32 OS_ObjectIdUnMap(uint32 id, uint32 idtype, uint32 *idvalue); - /*---------------------------------------------------------------- Function: OS_ObjectIdFindByName diff --git a/src/ut-stubs/osapi-utstub-idmap.c b/src/ut-stubs/osapi-utstub-idmap.c index 2766d9402..c94b38c11 100644 --- a/src/ut-stubs/osapi-utstub-idmap.c +++ b/src/ut-stubs/osapi-utstub-idmap.c @@ -52,49 +52,6 @@ void OS_Unlock_Global(uint32 idtype) UT_DEFAULT_IMPL(OS_Unlock_Global); } -/***************************************************************************** - * - * Stub function for OS_ObjectIdMap() - * - *****************************************************************************/ -int32 OS_ObjectIdMap(uint32 idtype, uint32 idvalue, uint32 *result) -{ - int32 Status; - - Status = UT_DEFAULT_IMPL(OS_ObjectIdMap); - - if (Status == 0 && - UT_Stub_CopyToLocal(UT_KEY(OS_ObjectIdMap), result, sizeof(*result)) < sizeof(*result)) - { - /* this needs to output something valid or code will break */ - *result = idvalue; - UT_FIXUP_ID(*result, idtype); - } - - return Status; -} - -/***************************************************************************** - * - * Stub function for OS_ObjectIdUnMap() - * - *****************************************************************************/ -int32 OS_ObjectIdUnMap(uint32 id, uint32 idtype, uint32 *idvalue) -{ - int32 Status; - - Status = UT_DEFAULT_IMPL(OS_ObjectIdUnMap); - - if (Status == 0 && - UT_Stub_CopyToLocal(UT_KEY(OS_ObjectIdUnMap), idvalue, sizeof(*idvalue)) < sizeof(*idvalue)) - { - /* this needs to output something valid or code will break */ - *idvalue = id & 0xFFFF; - } - - return Status; -} - /***************************************************************************** * * Stub function for OS_GetMaxForObjectType() From 1bf0574b2952564ca14a98721b75a2667fe098e2 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Tue, 18 Aug 2020 16:02:36 -0400 Subject: [PATCH 2/4] Fix #565, propagate return code from OS_TaskRegister_Impl() If this routine fails then return the error to the caller, which will also prevent the task from starting. --- src/os/shared/src/osapi-task.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/os/shared/src/osapi-task.c b/src/os/shared/src/osapi-task.c index 912b0ff86..bb78f1623 100644 --- a/src/os/shared/src/osapi-task.c +++ b/src/os/shared/src/osapi-task.c @@ -116,9 +116,10 @@ static int32 OS_TaskPrepare(uint32 task_id, osal_task_entry *entrypt) if (return_code == OS_SUCCESS) { - OS_TaskRegister_Impl(task_id); + return_code = OS_TaskRegister_Impl(task_id); } - else + + if (return_code != OS_SUCCESS) { *entrypt = NULL; } From bbb0dffc62d575fdab49ef25faa019df19e0213d Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Tue, 18 Aug 2020 15:15:05 -0400 Subject: [PATCH 3/4] Fix #564, consolidate delete finalization code Remove repetitive clearing of the global ID and unlocking global table and replace with common implementation in the idmap code. In addition to removing repeated logic, this makes deletion more like an inverse of creation, and provides a common/single point where additiona deletion-related logic can be added. --- src/os/shared/inc/os-shared-idmap.h | 13 ++++++++++ src/os/shared/src/osapi-binsem.c | 10 ++------ src/os/shared/src/osapi-countsem.c | 10 ++------ src/os/shared/src/osapi-dir.c | 10 ++------ src/os/shared/src/osapi-file.c | 10 ++------ src/os/shared/src/osapi-idmap.c | 22 +++++++++++++++++ src/os/shared/src/osapi-module.c | 10 ++------ src/os/shared/src/osapi-mutex.c | 10 ++------ src/os/shared/src/osapi-queue.c | 10 ++------ src/os/shared/src/osapi-task.c | 24 ++++++------------- src/os/shared/src/osapi-time.c | 6 ++--- src/os/shared/src/osapi-timebase.c | 10 ++------ .../shared/src/coveragetest-task.c | 5 ++-- src/ut-stubs/osapi-utstub-idmap.c | 15 ++++++++++++ 14 files changed, 77 insertions(+), 88 deletions(-) diff --git a/src/os/shared/inc/os-shared-idmap.h b/src/os/shared/inc/os-shared-idmap.h index 2be4d37ec..663f2438f 100644 --- a/src/os/shared/inc/os-shared-idmap.h +++ b/src/os/shared/inc/os-shared-idmap.h @@ -296,6 +296,19 @@ int32 OS_ObjectIdAllocateNew(uint32 idtype, const char *name, uint32 *array_inde ------------------------------------------------------------------*/ int32 OS_ObjectIdFinalizeNew(int32 operation_status, OS_common_record_t *record, uint32 *outid); +/*---------------------------------------------------------------- + Function: OS_ObjectIdFinalizeDelete + + Purpose: Completes a delete operation + If the operation was successful, the OSAL ID is deleted and returned to the pool + If the operation was unsuccessful, no operation is performed. + The global table is unlocked for future operations + + Returns: OS_SUCCESS on success, or relevant error code + ------------------------------------------------------------------*/ +int32 OS_ObjectIdFinalizeDelete(int32 operation_status, OS_common_record_t *record); + + /*---------------------------------------------------------------- Function: OS_ObjectIdRefcountDecr diff --git a/src/os/shared/src/osapi-binsem.c b/src/os/shared/src/osapi-binsem.c index 8fcd41b6e..3784aa652 100644 --- a/src/os/shared/src/osapi-binsem.c +++ b/src/os/shared/src/osapi-binsem.c @@ -157,14 +157,8 @@ int32 OS_BinSemDelete (uint32 sem_id) { return_code = OS_BinSemDelete_Impl(local_id); - /* Free the entry in the master table now while still locked */ - if (return_code == OS_SUCCESS) - { - /* Only need to clear the ID as zero is the "unused" flag */ - record->active_id = 0; - } - - OS_Unlock_Global(LOCAL_OBJID_TYPE); + /* Complete the operation via the common routine */ + return_code = OS_ObjectIdFinalizeDelete(return_code, record); } return return_code; diff --git a/src/os/shared/src/osapi-countsem.c b/src/os/shared/src/osapi-countsem.c index 01b2b4520..42f61d410 100644 --- a/src/os/shared/src/osapi-countsem.c +++ b/src/os/shared/src/osapi-countsem.c @@ -150,14 +150,8 @@ int32 OS_CountSemDelete (uint32 sem_id) { return_code = OS_CountSemDelete_Impl(local_id); - /* Free the entry in the master table now while still locked */ - if (return_code == OS_SUCCESS) - { - /* Only need to clear the ID as zero is the "unused" flag */ - record->active_id = 0; - } - - OS_Unlock_Global(LOCAL_OBJID_TYPE); + /* Complete the operation via the common routine */ + return_code = OS_ObjectIdFinalizeDelete(return_code, record); } return return_code; diff --git a/src/os/shared/src/osapi-dir.c b/src/os/shared/src/osapi-dir.c index e17b9b6c5..fe799bb5b 100644 --- a/src/os/shared/src/osapi-dir.c +++ b/src/os/shared/src/osapi-dir.c @@ -187,14 +187,8 @@ int32 OS_DirectoryClose(uint32 dir_id) { return_code = OS_DirClose_Impl(local_id); - /* Free the entry in the master table now while still locked */ - if (return_code == OS_SUCCESS) - { - /* Only need to clear the ID as zero is the "unused" flag */ - record->active_id = 0; - } - - OS_Unlock_Global(LOCAL_OBJID_TYPE); + /* Complete the operation via the common routine */ + return_code = OS_ObjectIdFinalizeDelete(return_code, record); } return return_code; diff --git a/src/os/shared/src/osapi-file.c b/src/os/shared/src/osapi-file.c index 88780a208..007c25bb6 100644 --- a/src/os/shared/src/osapi-file.c +++ b/src/os/shared/src/osapi-file.c @@ -223,14 +223,8 @@ int32 OS_close (uint32 filedes) { return_code = OS_GenericClose_Impl(local_id); - /* Free the entry in the master table now while still locked */ - if (return_code == OS_SUCCESS) - { - /* Only need to clear the ID as zero is the "unused" flag */ - record->active_id = 0; - } - - OS_Unlock_Global(LOCAL_OBJID_TYPE); + /* Complete the operation via the common routine */ + return_code = OS_ObjectIdFinalizeDelete(return_code, record); } return return_code; diff --git a/src/os/shared/src/osapi-idmap.c b/src/os/shared/src/osapi-idmap.c index cb4b52563..2d172bf3d 100644 --- a/src/os/shared/src/osapi-idmap.c +++ b/src/os/shared/src/osapi-idmap.c @@ -769,6 +769,28 @@ int32 OS_ObjectIdFinalizeNew(int32 operation_status, OS_common_record_t *record, return operation_status; } /* end OS_ObjectIdFinalizeNew */ +/*---------------------------------------------------------------- + Function: OS_ObjectIdFinalizeDelete + + Purpose: Helper routine, not part of OSAL public API. + See description in prototype + ------------------------------------------------------------------*/ +int32 OS_ObjectIdFinalizeDelete(int32 operation_status, OS_common_record_t *record) +{ + uint32 idtype = OS_ObjectIdToType_Impl(record->active_id); + + /* Clear the OSAL ID if successful - this returns the record to the pool */ + if (operation_status == OS_SUCCESS) + { + record->active_id = 0; + } + + /* Either way we must unlock the object type */ + OS_Unlock_Global(idtype); + + return operation_status; +} + /*---------------------------------------------------------------- * diff --git a/src/os/shared/src/osapi-module.c b/src/os/shared/src/osapi-module.c index bf8daff18..e730ac7cb 100644 --- a/src/os/shared/src/osapi-module.c +++ b/src/os/shared/src/osapi-module.c @@ -289,14 +289,8 @@ int32 OS_ModuleUnload ( uint32 module_id ) */ return_code = OS_ModuleUnload_Impl(local_id); - if (return_code == OS_SUCCESS) - { - /* Clear the ID to zero */ - record->active_id = 0; - } - - /* Unlock the global from OS_ObjectIdGetAndLock() */ - OS_Unlock_Global(LOCAL_OBJID_TYPE); + /* Complete the operation via the common routine */ + return_code = OS_ObjectIdFinalizeDelete(return_code, record); } return return_code; diff --git a/src/os/shared/src/osapi-mutex.c b/src/os/shared/src/osapi-mutex.c index 4f2bce392..eb35e079e 100644 --- a/src/os/shared/src/osapi-mutex.c +++ b/src/os/shared/src/osapi-mutex.c @@ -145,14 +145,8 @@ int32 OS_MutSemDelete (uint32 sem_id) { return_code = OS_MutSemDelete_Impl(local_id); - /* Free the entry in the master table now while still locked */ - if (return_code == OS_SUCCESS) - { - /* Only need to clear the ID as zero is the "unused" flag */ - record->active_id = 0; - } - - OS_Unlock_Global(LOCAL_OBJID_TYPE); + /* Complete the operation via the common routine */ + return_code = OS_ObjectIdFinalizeDelete(return_code, record); } return return_code; diff --git a/src/os/shared/src/osapi-queue.c b/src/os/shared/src/osapi-queue.c index 7cfdcee50..b1e104152 100644 --- a/src/os/shared/src/osapi-queue.c +++ b/src/os/shared/src/osapi-queue.c @@ -156,14 +156,8 @@ int32 OS_QueueDelete (uint32 queue_id) { return_code = OS_QueueDelete_Impl(local_id); - /* Free the entry in the master table now while still locked */ - if (return_code == OS_SUCCESS) - { - /* Only need to clear the ID as zero is the "unused" flag */ - record->active_id = 0; - } - - OS_Unlock_Global(LOCAL_OBJID_TYPE); + /* Complete the operation via the common routine */ + return_code = OS_ObjectIdFinalizeDelete(return_code, record); } return return_code; diff --git a/src/os/shared/src/osapi-task.c b/src/os/shared/src/osapi-task.c index 912b0ff86..663a74334 100644 --- a/src/os/shared/src/osapi-task.c +++ b/src/os/shared/src/osapi-task.c @@ -45,6 +45,7 @@ * User defined include files */ #include "os-shared-task.h" +#include "os-shared-common.h" #include "os-shared-idmap.h" @@ -116,7 +117,7 @@ static int32 OS_TaskPrepare(uint32 task_id, osal_task_entry *entrypt) if (return_code == OS_SUCCESS) { - OS_TaskRegister_Impl(task_id); + return_code = OS_TaskRegister_Impl(task_id); } else { @@ -269,24 +270,14 @@ int32 OS_TaskDelete (uint32 task_id) return_code = OS_TaskDelete_Impl(local_id); - /* Free the entry in the master table now while still locked */ - if (return_code == OS_SUCCESS) - { - /* Only need to clear the ID as zero is the "unused" flag */ - record->active_id = 0; - } - else - { - delete_hook = NULL; - } - - OS_Unlock_Global(LOCAL_OBJID_TYPE); + /* Complete the operation via the common routine */ + return_code = OS_ObjectIdFinalizeDelete(return_code, record); } /* ** Call the thread Delete hook if there is one. */ - if (delete_hook != NULL) + if (return_code == OS_SUCCESS && delete_hook != NULL) { delete_hook(); } @@ -312,9 +303,8 @@ void OS_TaskExit() task_id = OS_TaskGetId_Impl(); if (OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, LOCAL_OBJID_TYPE, task_id, &local_id, &record) == OS_SUCCESS) { - /* Only need to clear the ID as zero is the "unused" flag */ - record->active_id = 0; - OS_Unlock_Global(LOCAL_OBJID_TYPE); + /* Complete the operation via the common routine */ + OS_ObjectIdFinalizeDelete(OS_SUCCESS, record); } /* call the implementation */ diff --git a/src/os/shared/src/osapi-time.c b/src/os/shared/src/osapi-time.c index 646454e3b..31b76c8df 100644 --- a/src/os/shared/src/osapi-time.c +++ b/src/os/shared/src/osapi-time.c @@ -459,12 +459,10 @@ int32 OS_TimerDelete(uint32 timer_id) local->next_ref = local_id; local->prev_ref = local_id; - /* Clear the ID to zero */ - record->active_id = 0; - OS_TimeBaseUnlock_Impl(local->timebase_ref); - OS_Unlock_Global(OS_OBJECT_TYPE_OS_TIMECB); + /* Complete the operation via the common routine */ + return_code = OS_ObjectIdFinalizeDelete(return_code, record); } /* diff --git a/src/os/shared/src/osapi-timebase.c b/src/os/shared/src/osapi-timebase.c index 1ed2dfb5a..44b5d9f97 100644 --- a/src/os/shared/src/osapi-timebase.c +++ b/src/os/shared/src/osapi-timebase.c @@ -260,14 +260,8 @@ int32 OS_TimeBaseDelete(uint32 timer_id) { return_code = OS_TimeBaseDelete_Impl(local_id); - /* Free the entry in the master table now while still locked */ - if (return_code == OS_SUCCESS) - { - /* Clear the ID to zero */ - record->active_id = 0; - } - - OS_Unlock_Global(OS_OBJECT_TYPE_OS_TIMEBASE); + /* Complete the operation via the common routine */ + return_code = OS_ObjectIdFinalizeDelete(return_code, record); } return return_code; diff --git a/src/unit-test-coverage/shared/src/coveragetest-task.c b/src/unit-test-coverage/shared/src/coveragetest-task.c index 96b6c2225..0a01acb8c 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-task.c +++ b/src/unit-test-coverage/shared/src/coveragetest-task.c @@ -167,9 +167,8 @@ void Test_OS_TaskExit(void) OS_TaskExit(); - /* TaskExit should have cleared the active_id */ - UtAssert_True(utrec.active_id == 0, "utrec.active_id (%lu) == 0", - (unsigned long)utrec.active_id); + /* TaskExit should have called OS_ObjectIdFinalizeDelete to clear the active_id */ + UtAssert_STUB_COUNT(OS_ObjectIdFinalizeDelete, 1); } void Test_OS_TaskDelay(void) { diff --git a/src/ut-stubs/osapi-utstub-idmap.c b/src/ut-stubs/osapi-utstub-idmap.c index 2766d9402..d9ae7b64a 100644 --- a/src/ut-stubs/osapi-utstub-idmap.c +++ b/src/ut-stubs/osapi-utstub-idmap.c @@ -179,6 +179,21 @@ int32 OS_ObjectIdFinalizeNew(int32 operation_status, OS_common_record_t *record, return Status; } +/***************************************************************************** + * + * Stub function for OS_ObjectIdFinalizeDelete() + * + *****************************************************************************/ +int32 OS_ObjectIdFinalizeDelete(int32 operation_status, OS_common_record_t *record) +{ + int32 Status; + + Status = UT_DEFAULT_IMPL_RC(OS_ObjectIdFinalizeDelete, operation_status); + + return Status; +} + + /***************************************************************************** * * Stub function for OS_ObjectIdFindMatch() From a1d09b286ee15cca7fdbe2ebef9287a0266b6956 Mon Sep 17 00:00:00 2001 From: Yasir Khan Date: Tue, 25 Aug 2020 11:40:49 -0400 Subject: [PATCH 4/4] Increase version to 5.1.0-rc1+dev12, update readme Refactored +dev in OS_VERSION macros --- README.md | 9 ++++++++- src/os/inc/osapi-version.h | 6 +++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 96e67f1cb..0dffef494 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,14 @@ The autogenerated OSAL user's guide can be viewed at + ### Development Build: 5.1.0-rc1+dev5 - Adds OSAL network APIs missing functional tests as well as tests for OS_TimedRead and OS_TimedWrite @@ -30,7 +38,6 @@ The autogenerated OSAL user's guide can be viewed at ### Development Build: 5.0.21 diff --git a/src/os/inc/osapi-version.h b/src/os/inc/osapi-version.h index 1b6a36ae3..1e00754a1 100644 --- a/src/os/inc/osapi-version.h +++ b/src/os/inc/osapi-version.h @@ -30,8 +30,8 @@ /* * Development Build Macro Definitions */ -#define OS_BUILD_NUMBER 5 -#define OS_BUILD_BASELINE "v5.1.0-rc1+dev" +#define OS_BUILD_NUMBER 12 +#define OS_BUILD_BASELINE "v5.1.0-rc1" /* * Version Macro Definitions @@ -51,7 +51,7 @@ * @details Baseline git tag + Number of commits since baseline. @n * See @ref cfsversions for format differences between development and release versions. */ -#define OS_VERSION OS_BUILD_BASELINE OS_STR(OS_BUILD_NUMBER) +#define OS_VERSION OS_BUILD_BASELINE "+dev" OS_STR(OS_BUILD_NUMBER) /*! @brief Development Build Version String. * @details Reports the current development build's baseline, number, and name. Also includes a note about the latest official version. @n