Fix #870, generic counter table management

fsw/cfe-core/src/es/cfe_es_api.c

@@ -1507,27 +1507,31 @@ int32 CFE_ES_RegisterGenCounter(uint32 *CounterIdPtr, const char *CounterName)
    uint32 CheckPtr;
    int32 Status;
    uint32 i;
+   CFE_ES_GenCounterRecord_t *CountRecPtr;
 
    Status = CFE_ES_GetGenCounterIDByName(&CheckPtr, CounterName);
 
    if ((CounterIdPtr != NULL) && (CounterName != NULL) && (Status != CFE_SUCCESS))
    {
+      CFE_ES_LockSharedData(__func__,__LINE__);
+      CountRecPtr = CFE_ES_Global.CounterTable;
       for ( i = 0; i < CFE_PLATFORM_ES_MAX_GEN_COUNTERS; i++ )
       {
-         if ( CFE_ES_Global.CounterTable[i].RecordUsed == false )
+         if ( !CFE_ES_CounterRecordIsUsed(CountRecPtr) )
          {
-            strncpy((char *)CFE_ES_Global.CounterTable[i].CounterName,CounterName,OS_MAX_API_NAME);
-
-            CFE_ES_Global.CounterTable[i].RecordUsed = true;
-            CFE_ES_Global.CounterTable[i].Counter = 0;
-            *CounterIdPtr = i;
+            strncpy(CountRecPtr->CounterName,CounterName,OS_MAX_API_NAME);
+            CountRecPtr->Counter = 0;
+            CFE_ES_CounterRecordSetUsed(CountRecPtr, i);
+            *CounterIdPtr = CFE_ES_CounterRecordGetID(CountRecPtr);
             break;
          }
+         ++CountRecPtr;
       }
       if (i < CFE_PLATFORM_ES_MAX_GEN_COUNTERS)
       {
          ReturnCode = CFE_SUCCESS;
       }
+      CFE_ES_UnlockSharedData(__func__,__LINE__);
    }
 
    return ReturnCode;
@@ -1542,14 +1546,20 @@ int32 CFE_ES_RegisterGenCounter(uint32 *CounterIdPtr, const char *CounterName)
 */
 int32 CFE_ES_DeleteGenCounter(uint32 CounterId)
 {
-
+   CFE_ES_GenCounterRecord_t *CountRecPtr;
    int32 Status = CFE_ES_BAD_ARGUMENT;
 
-   if(CounterId < CFE_PLATFORM_ES_MAX_GEN_COUNTERS)
+   CountRecPtr = CFE_ES_LocateCounterRecordByID(CounterId);
+   if(CountRecPtr != NULL)
    {
-      CFE_ES_Global.CounterTable[CounterId].RecordUsed = false;
-      CFE_ES_Global.CounterTable[CounterId].Counter = 0;
-      Status = CFE_SUCCESS;
+      CFE_ES_LockSharedData(__func__,__LINE__);
+      if (CFE_ES_CounterRecordIsMatch(CountRecPtr, CounterId))
+      {
+          CountRecPtr->Counter = 0;
+          CFE_ES_CounterRecordSetFree(CountRecPtr);
+          Status = CFE_SUCCESS;
+      }
+      CFE_ES_UnlockSharedData(__func__,__LINE__);
    }
 
    return Status;
@@ -1565,12 +1575,13 @@ int32 CFE_ES_DeleteGenCounter(uint32 CounterId)
 int32 CFE_ES_IncrementGenCounter(uint32 CounterId)
 {
    int32 Status = CFE_ES_BAD_ARGUMENT;
+   CFE_ES_GenCounterRecord_t *CountRecPtr;
 
-   if((CounterId < CFE_PLATFORM_ES_MAX_GEN_COUNTERS) &&
-      (CFE_ES_Global.CounterTable[CounterId].RecordUsed == true))
+   CountRecPtr = CFE_ES_LocateCounterRecordByID(CounterId);
+   if(CFE_ES_CounterRecordIsMatch(CountRecPtr, CounterId))
    {
-      CFE_ES_Global.CounterTable[CounterId].Counter++;
-      Status = CFE_SUCCESS;
+       ++CountRecPtr->Counter;
+       Status = CFE_SUCCESS;
    }
    return Status;
 
@@ -1585,11 +1596,12 @@ int32 CFE_ES_IncrementGenCounter(uint32 CounterId)
 int32 CFE_ES_SetGenCount(uint32 CounterId, uint32 Count)
 {
    int32 Status = CFE_ES_BAD_ARGUMENT;
+   CFE_ES_GenCounterRecord_t *CountRecPtr;
 
-   if((CounterId < CFE_PLATFORM_ES_MAX_GEN_COUNTERS) &&
-      (CFE_ES_Global.CounterTable[CounterId].RecordUsed == true))
+   CountRecPtr = CFE_ES_LocateCounterRecordByID(CounterId);
+   if(CFE_ES_CounterRecordIsMatch(CountRecPtr, CounterId))
    {
-      CFE_ES_Global.CounterTable[CounterId].Counter = Count;
+      CountRecPtr->Counter = Count;
       Status = CFE_SUCCESS;
    }
    return Status;
@@ -1604,41 +1616,47 @@ int32 CFE_ES_SetGenCount(uint32 CounterId, uint32 Count)
 int32 CFE_ES_GetGenCount(uint32 CounterId, uint32 *Count)
 {
    int32 Status = CFE_ES_BAD_ARGUMENT;
+   CFE_ES_GenCounterRecord_t *CountRecPtr;
 
-   if((CounterId < CFE_PLATFORM_ES_MAX_GEN_COUNTERS) &&
-      (CFE_ES_Global.CounterTable[CounterId].RecordUsed == true) &&
-      (Count != NULL ))
+   CountRecPtr = CFE_ES_LocateCounterRecordByID(CounterId);
+   if(CFE_ES_CounterRecordIsMatch(CountRecPtr, CounterId) &&
+           Count != NULL)
    {
-      *Count = CFE_ES_Global.CounterTable[CounterId].Counter;
+      *Count = CountRecPtr->Counter;
       Status = CFE_SUCCESS;
    }
    return Status;
 } /* End of CFE_ES_GetGenCount() */
 
 int32 CFE_ES_GetGenCounterIDByName(uint32 *CounterIdPtr, const char *CounterName)
 {
-
+   CFE_ES_GenCounterRecord_t *CountRecPtr;
    int32 Result = CFE_ES_BAD_ARGUMENT;
    uint32   i;
 
    /*
    ** Search the ES Generic Counter table for a counter with a matching name.
    */
+   CFE_ES_LockSharedData(__func__,__LINE__);
+   CountRecPtr = CFE_ES_Global.CounterTable;
+
    for ( i = 0; i < CFE_PLATFORM_ES_MAX_GEN_COUNTERS; i++ )
    {
-      if ( CFE_ES_Global.CounterTable[i].RecordUsed == true )
+      if ( CFE_ES_CounterRecordIsUsed(CountRecPtr) )
       {
-         if ( strncmp(CounterName, (char *)CFE_ES_Global.CounterTable[i].CounterName, OS_MAX_API_NAME) == 0 )
+         if ( strncmp(CounterName, CountRecPtr->CounterName, OS_MAX_API_NAME) == 0 )
          {
             if(CounterIdPtr != NULL)
             {
-               *CounterIdPtr = i;
+               *CounterIdPtr = CFE_ES_CounterRecordGetID(CountRecPtr);
                Result = CFE_SUCCESS;
             }
             break;
          }
       }
+      ++CountRecPtr;
    } /* end for */
+   CFE_ES_UnlockSharedData(__func__,__LINE__);
 
    return(Result);
 
@@ -1681,6 +1699,25 @@ int32 CFE_ES_TaskID_ToIndex(uint32 TaskID, uint32 *Idx)
     return CFE_SUCCESS;
 }
 
+/*
+ * A conversion function to obtain an index value correlating to a CounterID
+ * This is a zero based value that can be used for indexing into a table.
+ */
+int32 CFE_ES_CounterID_ToIndex(uint32 CounterId, uint32 *Idx)
+{
+    if (CounterId >= CFE_PLATFORM_ES_MAX_GEN_COUNTERS)
+    {
+        return CFE_ES_BAD_ARGUMENT; /* these do not have a dedicated error */
+    }
+
+    /*
+     * Currently this is a direct/simple pass through.
+     * Will evolve in a future rev to make it more safe.
+     */
+    *Idx = CounterId;
+    return CFE_SUCCESS;
+}
+
 /***************************************************************************************
 ** Private API functions
 */
@@ -1729,6 +1766,24 @@ CFE_ES_TaskRecord_t *CFE_ES_LocateTaskRecordByID(uint32 TaskID)
     return TaskRecPtr;
 }
 
+CFE_ES_GenCounterRecord_t* CFE_ES_LocateCounterRecordByID(uint32 CounterID)
+{
+    CFE_ES_GenCounterRecord_t *CounterRecPtr;
+    uint32 Idx;
+
+    if (CFE_ES_CounterID_ToIndex(CounterID, &Idx) == CFE_SUCCESS)
+    {
+        CounterRecPtr = &CFE_ES_Global.CounterTable[Idx];
+    }
+    else
+    {
+        CounterRecPtr = NULL;
+    }
+
+    return CounterRecPtr;
+}
+
+
 /*
  * This function does additional validation on the task record
  * and should only be called when global data is locked.

fsw/cfe-core/src/es/cfe_es_global.h

@@ -177,6 +177,17 @@ extern CFE_ES_AppRecord_t* CFE_ES_LocateAppRecordByID(uint32 AppID);
  */
 extern CFE_ES_TaskRecord_t* CFE_ES_LocateTaskRecordByID(uint32 TaskID);
 
+/**
+ * @brief Locate the Counter table entry correlating with a given Counter ID.
+ *
+ * This only returns a pointer to the table entry and does _not_
+ * otherwise check/validate the entry.
+ *
+ * @param[in]   CounterID   the Counter ID to locate
+ * @return pointer to Counter Table entry for the given Counter ID
+ */
+extern CFE_ES_GenCounterRecord_t* CFE_ES_LocateCounterRecordByID(uint32 CounterID);
+
 /**
  * @brief Check if an app record is in use or free/empty
  *
@@ -348,6 +359,91 @@ static inline bool CFE_ES_TaskRecordIsMatch(const CFE_ES_TaskRecord_t *TaskRecPt
             CFE_ES_TaskRecordGetID(TaskRecPtr) == TaskID);
 }
 
+/**
+ * @brief Check if an Counter record is in use or free/empty
+ *
+ * This routine checks if the Counter table entry is in use or if it is free
+ *
+ * As this dereferences fields within the record, global data must be
+ * locked prior to invoking this function.
+ *
+ * @param[in]   CounterRecPtr   pointer to Counter table entry
+ * @returns true if the entry is in use/configured, or false if it is free/empty
+ */
+static inline bool CFE_ES_CounterRecordIsUsed(const CFE_ES_GenCounterRecord_t *CounterRecPtr)
+{
+    return (CounterRecPtr->RecordUsed);
+}
+
+/**
+ * @brief Get the ID value from an Counter table entry
+ *
+ * This routine converts the table entry back to an abstract ID.
+ *
+ * @param[in]   CounterRecPtr   pointer to Counter table entry
+ * @returns CounterID of entry
+ */
+static inline uint32 CFE_ES_CounterRecordGetID(const CFE_ES_GenCounterRecord_t *CounterRecPtr)
+{
+    /*
+     * The initial implementation does not store the ID in the entry;
+     * the ID is simply the zero-based index into the table.
+     */
+    return (CounterRecPtr - CFE_ES_Global.CounterTable);
+}
+
+/**
+ * @brief Marks an Counter table entry as used (not free)
+ *
+ * This sets the internal field(s) within this entry, and marks
+ * it as being associated with the given Counter ID.
+ *
+ * As this dereferences fields within the record, global data must be
+ * locked prior to invoking this function.
+ *
+ * @param[in]   CounterRecPtr   pointer to Counter table entry
+ * @param[in]   CounterID       the Counter ID of this entry
+ */
+static inline void CFE_ES_CounterRecordSetUsed(CFE_ES_GenCounterRecord_t *CounterRecPtr, uint32 CounterID)
+{
+    CounterRecPtr->RecordUsed = true;
+}
+
+/**
+ * @brief Set an Counter record table entry free (not used)
+ *
+ * This clears the internal field(s) within this entry, and allows the
+ * memory to be re-used in the future.
+ *
+ * As this dereferences fields within the record, global data must be
+ * locked prior to invoking this function.
+ *
+ * @param[in]   CounterRecPtr   pointer to Counter table entry
+ */
+static inline void CFE_ES_CounterRecordSetFree(CFE_ES_GenCounterRecord_t *CounterRecPtr)
+{
+    CounterRecPtr->RecordUsed = false;
+}
+
+/**
+ * @brief Check if an Counter record is a match for the given CounterID
+ *
+ * This routine confirms that the previously-located record is valid
+ * and matches the expected Counter ID.
+ *
+ * As this dereferences fields within the record, global data must be
+ * locked prior to invoking this function.
+ *
+ * @param[in]   CounterRecPtr   pointer to Counter table entry
+ * @param[in]   CounterID       expected Counter ID
+ * @returns true if the entry matches the given Counter ID
+ */
+static inline bool CFE_ES_CounterRecordIsMatch(const CFE_ES_GenCounterRecord_t *CounterRecPtr, uint32 CounterID)
+{
+    return (CounterRecPtr != NULL && CFE_ES_CounterRecordIsUsed(CounterRecPtr) &&
+            CFE_ES_CounterRecordGetID(CounterRecPtr) == CounterID);
+}
+
 /**
  * Locate and validate the app record for the calling context.
  *

fsw/cfe-core/src/es/cfe_es_start.c

@@ -87,6 +87,7 @@ void CFE_ES_Main(uint32 StartType, uint32 StartSubtype, uint32 ModeId, const cha
    int32 ReturnCode;
    CFE_ES_AppRecord_t *AppRecPtr;
    CFE_ES_TaskRecord_t *TaskRecPtr;
+   CFE_ES_GenCounterRecord_t *CountRecPtr;
 
    /*
    ** Indicate that the CFE is the earliest initialization state
@@ -200,9 +201,11 @@ void CFE_ES_Main(uint32 StartType, uint32 StartSubtype, uint32 ModeId, const cha
    ** Initialize the ES Generic Counter Table
    ** to mark all entries as unused.
    */
+   CountRecPtr = CFE_ES_Global.CounterTable;
    for ( i = 0; i < CFE_PLATFORM_ES_MAX_GEN_COUNTERS; i++ )
    {
-      CFE_ES_Global.CounterTable[i].RecordUsed = false;
+       CFE_ES_CounterRecordSetFree(CountRecPtr);
+       ++CountRecPtr;
    }
 
    /*

fsw/cfe-core/unit-test/es_UT.c

@@ -4741,11 +4741,6 @@ void TestGenericCounterAPI(void)
     /* Test registering a generic counter with a null counter name */
     ES_ResetUnitTest();
 
-    for ( i = 0; i < CFE_PLATFORM_ES_MAX_GEN_COUNTERS; i++ )
-    {
-       CFE_ES_Global.CounterTable[i].RecordUsed = false;
-    }
-
     UT_Report(__FILE__, __LINE__,
              CFE_ES_RegisterGenCounter(&CounterId,
                                         NULL) == CFE_ES_BAD_ARGUMENT,
@@ -4761,15 +4756,13 @@ void TestGenericCounterAPI(void)
 
     /* Test setting a generic counter where the record is not in use */
     ES_ResetUnitTest();
-    CFE_ES_Global.CounterTable[CounterId].RecordUsed = false;
     UT_Report(__FILE__, __LINE__,
              CFE_ES_SetGenCount(CounterId, 0) == CFE_ES_BAD_ARGUMENT,
               "CFE_ES_SetGenCount",
               "Record not in use");
 
     /* Test getting a generic counter where the record is not in use */
     ES_ResetUnitTest();
-    CFE_ES_Global.CounterTable[CounterId].RecordUsed = false;
     UT_Report(__FILE__, __LINE__,
              CFE_ES_GetGenCount(CounterId, &CounterCount)
                 == CFE_ES_BAD_ARGUMENT,
@@ -4778,7 +4771,6 @@ void TestGenericCounterAPI(void)
 
     /* Test getting a generic counter where the count is null */
     ES_ResetUnitTest();
-    CFE_ES_Global.CounterTable[CounterId].RecordUsed = true;
     UT_Report(__FILE__, __LINE__,
              CFE_ES_GetGenCount(CounterId, NULL)
                 == CFE_ES_BAD_ARGUMENT,