Merge pull request #127 from jphickey/fix-116-117-118

Fix #116, 117+118 - Multiple cleanups related to array indexing and range checking

config/default_sc_extern_typedefs.h

@@ -0,0 +1,131 @@
+/************************************************************************
+ * NASA Docket No. GSC-18,924-1, and identified as “Core Flight
+ * System (cFS) Stored Command Application version 3.1.1”
+ *
+ * Copyright (c) 2021 United States Government as represented by the
+ * Administrator of the National Aeronautics and Space Administration.
+ * All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License. You may obtain
+ * a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ ************************************************************************/
+
+/**
+ * @file
+ * Definition of CFS Stored Command (SC) types that are shared between
+ * tables, messages, or otherwise need to be known publicly.
+ */
+#ifndef SC_EXTERN_TYPEDEFS_H
+#define SC_EXTERN_TYPEDEFS_H
+
+#include "common_types.h"
+
+/**
+ * @brief An identifier for RTS's
+ *
+ * This is a 1-based numeric value that refers to a specific RTS.
+ * The value of 0 is reserved and is considered invalid/null.
+ *
+ * The valid range is [1..SC_NUMBER_OF_RTS] (inclusive)
+ *
+ * @note Some code and documentation may also refer to this as an RTS Number.
+ * This is synonymous with an RTS ID.
+ */
+typedef uint16 SC_RtsNum_t;
+
+/**
+ * @brief An identifier for ATS's
+ *
+ * This is a 1-based numeric value that refers to a specific ATS.
+ * The value of 0 is reserved and is considered invalid/null.
+ *
+ * The valid range is [1..SC_NUMBER_OF_ATS] (inclusive)
+ *
+ * @note Some code and documentation may also refer to this as an ATS Number.
+ * This is synonymous with an ATS ID.
+ *
+ * Unlike RTS, in many circumstances an alphabetic identifier is also used
+ * to identify an ATS (e.g. ATS A, ATS B, etc).  This is a simple mapping where
+ * A refers to ATS ID 1, B refers to ATS ID 2, etc.
+ */
+typedef uint16 SC_AtsNum_t;
+
+/**
+ * A command number for ATS's
+ *
+ * This is a 1-based numeric value that refers to a specific
+ * command within an ATS.  Each entry within an ATS has one of
+ * these identifiers on each command in it.
+ *
+ * @note RTS sequences do not use this identifier, as these
+ * commands only have a relative offset from the previous command.
+ *
+ * The value of 0 is reserved and is considered invalid/null.
+ *
+ * The valid range is [1..SC_MAX_ATS_CMDS] (inclusive)
+ *
+ * IMPORTANT: This number only serves to uniquely identify a
+ * specific command within an ATS.  It is _not_ necessarily the
+ * same as a sequence number within the ATS, as commands may be
+ * defined in the table any order (that is, they may have absolute
+ * time stamps that are not in sequence).
+ */
+typedef uint16 SC_CommandNum_t;
+
+/**
+ * @brief Represents an offset into an ATS or RTS buffer
+ *
+ * This is a 0-based numeric value that refers to a 32-bit word position
+ * within the ATS or RTS buffer.  This can be used to look up the
+ * specific command at that position.
+ *
+ * The valid range is [0..(SC_ATS_BUFF_SIZE/4)-1] for ATS
+ * or [0..(SC_RTS_BUFF_SIZE/4)-1] for RTS
+ *
+ * @note ATS/RTS Buffers are indexed using 32-bit words.
+ * To get a byte offset, this value needs to be multiplied by 4.
+ */
+typedef uint16 SC_EntryOffset_t;
+
+/**
+ * Convert from an ID or Number value (e.g. RTS/ATS identifier) to a native unsigned int
+ *
+ * This is mainly intended for printf()-style logging, where it should be paired
+ * with the "%u" conversion specifier.
+ */
+#define SC_IDNUM_AS_UINT(arg) ((unsigned int)(arg))
+
+/**
+ * Convert from a native integer value (e.g. a literal) to an ID or Number value
+ *
+ * This is mainly intended for initializing values from literals or integers
+ * This is the inverse macro of SC_IDNUM_AS_UINT()
+ */
+#define SC_IDNUM_FROM_UINT(arg) ((uint16)(arg))
+
+/* _INITIALIZER macros to be used in static (e.g. table) definitions that need to resolve at compile time */
+#define SC_RTS_NUM_INITIALIZER(i)     SC_IDNUM_FROM_UINT(i)
+#define SC_ATS_NUM_INITIALIZER(i)     SC_IDNUM_FROM_UINT(i)
+#define SC_COMMAND_NUM_INITIALIZER(i) SC_IDNUM_FROM_UINT(i)
+
+#define SC_IDNUM_EQUAL(arg1, arg2) (SC_IDNUM_AS_UINT(arg1) == SC_IDNUM_AS_UINT(arg2))
+#define SC_IDNUM_IS_NULL(arg)      (SC_IDNUM_AS_UINT(arg) == 0)
+
+/* _C macros to be used in other places that need to resolve at runtime time - these are type safe */
+#define SC_RTS_NUM_C(i)     ((SC_RtsNum_t)SC_IDNUM_FROM_UINT(i))
+#define SC_ATS_NUM_C(i)     ((SC_AtsNum_t)SC_IDNUM_FROM_UINT(i))
+#define SC_COMMAND_NUM_C(i) ((SC_CommandNum_t)SC_IDNUM_FROM_UINT(i))
+
+/* _NULL macros refer to a value that is always reserved */
+#define SC_RTS_NUM_NULL     SC_RTS_NUM_C(0)
+#define SC_ATS_NUM_NULL     SC_ATS_NUM_C(0)
+#define SC_COMMAND_NUM_NULL SC_COMMAND_NUM_C(0)
+
+#endif

config/default_sc_msgdefs.h

@@ -28,6 +28,7 @@
 #define SC_MSGDEFS_H
 
 #include "common_types.h"
+#include "sc_extern_typedefs.h"
 #include "sc_fcncodes.h"
 
 /************************************************************************
@@ -160,6 +161,7 @@ enum SC_AtsCont
     SC_AtsCont_FALSE = false, /**< \brief Do not continue on failure */
     SC_AtsCont_TRUE  = true   /**< \brief Continue on failure */
 };
+
 typedef uint8 SC_AtsCont_Enum_t;
 
 #ifndef SC_OMIT_DEPRECATED
@@ -194,28 +196,28 @@ typedef struct
     uint8 CmdCtr;    /**< \brief Counts Ground Requests */
     uint8 Padding8;  /**< \brief Structure padding */
 
-    uint16 SwitchPendFlag;  /**< \brief Switch pending flag: 0 = NO, 1 = YES */
-    uint16 NumRtsActive;    /**< \brief Number of RTSs currently active */
-    uint16 RtsNumber;       /**< \brief Next RTS number */
-    uint16 RtsActiveCtr;    /**< \brief Increments when an RTS is started without error */
-    uint16 RtsActiveErrCtr; /**< \brief Increments when an attempt to start an RTS fails */
-    uint16 AtsCmdCtr;       /**< \brief Total ATS cmd cnter counts commands sent by the ATS */
-    uint16 AtsCmdErrCtr;    /**< \brief Total ATS cmd Error ctr command errors in the ATS */
-    uint16 RtsCmdCtr;       /**< \brief Counts TOTAL rts cmds that were sent out from ALL active RTSs */
-    uint16 RtsCmdErrCtr;    /**< \brief Counts TOTAL number of errs from ALL RTSs that are active */
-    uint16 LastAtsErrSeq;   /**< \brief Last ATS Errant Sequence Num Values: 1 or 2 */
-    uint16 LastAtsErrCmd;   /**< \brief Last ATS Errant Command Num */
-    uint16 LastRtsErrSeq;   /**< \brief Last RTS Errant Sequence Num */
-    uint16 LastRtsErrCmd;   /**< \brief Offset in the RTS buffer for the last command error, in "words" */
-
-    uint16 AppendCmdArg;                   /**< \brief ATS selection argument from most recent Append ATS command */
-    uint16 AppendEntryCount;               /**< \brief Number of cmd entries in current Append ATS table */
-    uint16 AppendByteCount;                /**< \brief Size of cmd entries in current Append ATS table */
-    uint16 AppendLoadCount;                /**< \brief Total number of Append ATS table loads */
-    uint32 AtpCmdNumber;                   /**< \brief Current command number */
-    uint32 AtpFreeBytes[SC_NUMBER_OF_ATS]; /**< \brief Free Bytes in each ATS  */
-    uint32 NextRtsTime;                    /**< \brief Next RTS cmd Absolute Time */
-    uint32 NextAtsTime;                    /**< \brief Next ATS Command Time (seconds) */
+    uint16           SwitchPendFlag;  /**< \brief Switch pending flag: 0 = NO, 1 = YES */
+    uint16           NumRtsActive;    /**< \brief Number of RTSs currently active */
+    SC_RtsNum_t      RtsNum;          /**< \brief Next RTS number */
+    uint16           RtsActiveCtr;    /**< \brief Increments when an RTS is started without error */
+    uint16           RtsActiveErrCtr; /**< \brief Increments when an attempt to start an RTS fails */
+    uint16           AtsCmdCtr;       /**< \brief Total ATS cmd cnter counts commands sent by the ATS */
+    uint16           AtsCmdErrCtr;    /**< \brief Total ATS cmd Error ctr command errors in the ATS */
+    uint16           RtsCmdCtr;       /**< \brief Counts TOTAL rts cmds that were sent out from ALL active RTSs */
+    uint16           RtsCmdErrCtr;    /**< \brief Counts TOTAL number of errs from ALL RTSs that are active */
+    SC_AtsNum_t      LastAtsErrSeq;   /**< \brief Last ATS Errant Sequence Num Values: 1 or 2 */
+    SC_CommandNum_t  LastAtsErrCmd;   /**< \brief Last ATS Errant Command Num */
+    SC_RtsNum_t      LastRtsErrSeq;   /**< \brief Last RTS Errant Sequence Num */
+    SC_EntryOffset_t LastRtsErrCmd;   /**< \brief Offset in the RTS buffer for the last command error, in "words" */
+
+    SC_AtsNum_t AppendCmdArg;     /**< \brief ATS selection argument from most recent Append ATS command */
+    uint16      AppendEntryCount; /**< \brief Number of cmd entries in current Append ATS table */
+    uint16      AppendByteCount;  /**< \brief Size of cmd entries in current Append ATS table */
+    uint16      AppendLoadCount;  /**< \brief Total number of Append ATS table loads */
+    uint32      AtpCmdNumber;     /**< \brief Current command number */
+    uint32      AtpFreeBytes[SC_NUMBER_OF_ATS]; /**< \brief Free Bytes in each ATS  */
+    uint32      NextRtsTime;                    /**< \brief Next RTS cmd Absolute Time */
+    uint32      NextAtsTime;                    /**< \brief Next ATS Command Time (seconds) */
 
     uint16 RtsExecutingStatus[(SC_NUMBER_OF_RTS + (SC_NUMBER_OF_RTS_IN_UINT16 - 1)) / SC_NUMBER_OF_RTS_IN_UINT16];
     /**< \brief RTS executing status bit map where each uint16 represents 16 RTS numbers.  Note: array
@@ -244,17 +246,17 @@ typedef struct
  */
 typedef struct
 {
-    uint16 AtsId;   /**< \brief The ID of the ATS to start, 1 = ATS_A, 2 = ATS_B */
-    uint16 Padding; /**< \brief Structure padding */
+    SC_AtsNum_t AtsNum;  /**< \brief The ID of the ATS to start, 1 = ATS_A, 2 = ATS_B */
+    uint16      Padding; /**< \brief Structure padding */
 } SC_StartAtsCmd_Payload_t;
 
 /**
  *  \brief RTS Id Command Payload
  */
 typedef struct
 {
-    uint16 RtsId;   /**< \brief The ID of the RTS to start, 1 through #SC_NUMBER_OF_RTS */
-    uint16 Padding; /**< \brief Structure padding */
+    SC_RtsNum_t RtsNum;  /**< \brief The ID of the RTS to start, 1 through #SC_NUMBER_OF_RTS */
+    uint16      Padding; /**< \brief Structure padding */
 } SC_RtsCmd_Payload_t;
 
 /**
@@ -279,17 +281,17 @@ typedef struct
  */
 typedef struct
 {
-    uint16 AtsId;   /**< \brief The ID of the ATS to append to, 1 = ATS_A, 2 = ATS_B */
-    uint16 Padding; /**< \brief Structure Padding */
+    SC_AtsNum_t AtsNum;  /**< \brief The ID of the ATS to append to, 1 = ATS_A, 2 = ATS_B */
+    uint16      Padding; /**< \brief Structure Padding */
 } SC_AppendAtsCmd_Payload_t;
 
 /**
  *  \brief RTS Group Command Payload
  */
 typedef struct
 {
-    uint16 FirstRtsId; /**< \brief ID of the first RTS to act on, 1 through #SC_NUMBER_OF_RTS */
-    uint16 LastRtsId;  /**< \brief ID of the last RTS to act on, 1 through #SC_NUMBER_OF_RTS */
+    SC_RtsNum_t FirstRtsNum; /**< \brief ID of the first RTS to act on, 1 through #SC_NUMBER_OF_RTS */
+    SC_RtsNum_t LastRtsNum;  /**< \brief ID of the last RTS to act on, 1 through #SC_NUMBER_OF_RTS */
 } SC_RtsGrpCmd_Payload_t;
 
 /**\}*/

config/default_sc_tbldefs.h

@@ -27,6 +27,7 @@
 #define SC_TBLDEFS_H
 
 #include "common_types.h"
+#include "sc_extern_typedefs.h"
 #include "cfe_msg_hdr.h"
 
 /*************************************************************************
@@ -70,7 +71,7 @@ typedef struct
 {
     uint16 Pad; /**< \brief Structure padding */
 
-    uint16 CmdNumber; /**< \brief command number, range = 1 to SC_MAX_ATS_CMDS */
+    SC_CommandNum_t CmdNumber; /**< \brief command number, range = 1 to SC_MAX_ATS_CMDS */
 
     uint16 TimeTag_MS; /**< \brief Time tag most significant 16 bits */
     uint16 TimeTag_LS; /**< \brief Time tag least significant 16 bits */

fsw/src/sc_app.c

@@ -167,11 +167,11 @@ CFE_Status_t SC_AppInit(void)
     /* Select auto-exec RTS to start during first HK request */
     if (CFE_ES_GetResetType(NULL) == CFE_PSP_RST_TYPE_POWERON)
     {
-        SC_AppData.AutoStartRTS = RTS_ID_AUTO_POWER_ON;
+        SC_AppData.AutoStartRTS = SC_RTS_NUM_C(RTS_ID_AUTO_POWER_ON);
     }
     else
     {
-        SC_AppData.AutoStartRTS = RTS_ID_AUTO_PROCESSOR;
+        SC_AppData.AutoStartRTS = SC_RTS_NUM_C(RTS_ID_AUTO_PROCESSOR);
     }
 
     /* Must be able to register for events */
@@ -240,9 +240,11 @@ CFE_Status_t SC_AppInit(void)
 
 CFE_Status_t SC_InitTables(void)
 {
-    CFE_Status_t Result;
-    int32        i;
-    int32        j;
+    CFE_Status_t            Result;
+    int32                   i;
+    int32                   j;
+    SC_RtsInfoEntry_t *     RtsInfoPtr;
+    SC_AtsCmdStatusEntry_t *StatusEntryPtr;
 
     /* Must be able to register all tables with cFE Table Services */
     Result = SC_RegisterAllTables();
@@ -259,30 +261,34 @@ CFE_Status_t SC_InitTables(void)
     }
 
     /* ATP control block status table */
-    SC_OperData.AtsCtrlBlckAddr->AtpState  = SC_Status_IDLE;
-    SC_OperData.AtsCtrlBlckAddr->AtsNumber = SC_AtsId_NO_ATS;
-    SC_OperData.AtsCtrlBlckAddr->CmdNumber = SC_INVALID_CMD_NUMBER;
+    SC_OperData.AtsCtrlBlckAddr->AtpState   = SC_Status_IDLE;
+    SC_OperData.AtsCtrlBlckAddr->CurrAtsNum = SC_ATS_NUM_NULL;
+    SC_OperData.AtsCtrlBlckAddr->CmdNumber  = SC_INVALID_CMD_NUMBER;
 
     /* RTP control block status table */
     SC_OperData.RtsCtrlBlckAddr->NumRtsActive = 0;
-    SC_OperData.RtsCtrlBlckAddr->RtsNumber    = SC_INVALID_RTS_NUMBER;
+    SC_OperData.RtsCtrlBlckAddr->CurrRtsNum   = SC_RTS_NUM_NULL;
 
     /* ATS command status table(s) */
     for (i = 0; i < SC_NUMBER_OF_ATS; i++)
     {
         for (j = 0; j < SC_MAX_ATS_CMDS; j++)
         {
-            SC_OperData.AtsCmdStatusTblAddr[i][j] = SC_Status_EMPTY;
+            StatusEntryPtr = SC_GetAtsStatusEntryForCommand(SC_ATS_IDX_C(i), SC_COMMAND_IDX_C(j));
+
+            StatusEntryPtr->Status = SC_Status_EMPTY;
         }
     }
 
     /* RTS information table */
     for (i = 0; i < SC_NUMBER_OF_RTS; i++)
     {
-        SC_OperData.RtsInfoTblAddr[i].NextCommandTime = SC_MAX_TIME;
-        SC_OperData.RtsInfoTblAddr[i].NextCommandPtr  = 0;
-        SC_OperData.RtsInfoTblAddr[i].RtsStatus       = SC_Status_EMPTY;
-        SC_OperData.RtsInfoTblAddr[i].DisabledFlag    = true;
+        RtsInfoPtr = SC_GetRtsInfoObject(SC_RTS_IDX_C(i));
+
+        RtsInfoPtr->NextCommandTime = SC_MAX_TIME;
+        RtsInfoPtr->NextCommandPtr  = SC_ENTRY_OFFSET_FIRST;
+        RtsInfoPtr->RtsStatus       = SC_Status_EMPTY;
+        RtsInfoPtr->DisabledFlag    = true;
     }
 
     /* Load default RTS tables */
@@ -516,7 +522,7 @@ CFE_Status_t SC_GetLoadTablePointers(void)
         /* Process new RTS table data */
         if (Result == CFE_TBL_INFO_UPDATED)
         {
-            SC_LoadRts(i);
+            SC_LoadRts(SC_RTS_IDX_C(i));
         }
     }