Merge pull request #567 from jphickey/fix-564-consolidate-delete

Fix #564, consolidate delete finalization code

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
 

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;

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;

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;

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;

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;
+}
+
 
 /*----------------------------------------------------------------
  *

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;

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;

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;

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 */

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);
     }
 
     /*

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;

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)
 {

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()