Fix #1458, Moves OS_strnlen to public API and adds static analysis co…

…mments This commit addresses issues flagged during static analysis by: - Adding JSC 2.1 disposition comments. - Making OS_strnlen publicly accessible and replacing strlen with it.
nasa · Jun 24, 2024 · 3b40ede · 3b40ede
1 parent 6483329
commit 3b40ede
Show file tree

Hide file tree

Showing 10 changed files with 131 additions and 24 deletions.
diff --git a/src/os/inc/osapi-clock.h b/src/os/inc/osapi-clock.h
@@ -215,6 +215,7 @@ static inline int64 OS_TimeGetTotalMicroseconds(OS_time_t tm)
  */
 static inline OS_time_t OS_TimeFromTotalMicroseconds(int64 tm)
 {
+    /* SAD: Overflow is not considered a concern because tm would need to be over 29,227 years in microseconds */
     OS_time_t ostm = {tm * OS_TIME_TICKS_PER_USEC};
     return ostm;
 }

diff --git a/src/os/inc/osapi-common.h b/src/os/inc/osapi-common.h
@@ -233,6 +233,21 @@ void OS_ApplicationExit(int32 Status);
  */
 int32 OS_RegisterEventHandler(OS_EventHandler_t handler);
 
+/*-------------------------------------------------------------------------------------*/
+/**
+ * @brief get string length
+ *
+ * Provides an OSAL routine to get the functionality
+ * of the (non-C99) "strnlen()" function, via the
+ * C89/C99 standard "memchr()" function instead.
+ *
+ * @param[in]    s       The input string
+ * @param[in]    maxlen  Maximum length to check
+ * @retval Length of the string or maxlen, whichever is smaller.
+
+ */
+size_t OS_strnlen(const char *s, size_t maxlen);
+
 /**@}*/
 
 #endif /* OSAPI_COMMON_H */
diff --git a/src/os/shared/inc/os-shared-common.h b/src/os/shared/inc/os-shared-common.h
@@ -129,25 +129,4 @@ void OS_IdleLoop_Impl(void);
  ------------------------------------------------------------------*/
 void OS_ApplicationShutdown_Impl(void);
 
-/*----------------------------------------------------------------
-
-    Purpose: Utility function to safely find the length of a string
-             within a fixed-size array buffer.
-
-             Provides a local OSAL routine to get the functionality
-             of the (non-C99) "strnlen()" function, via the
-             C89/C99 standard "memchr()" function instead.
-
- ------------------------------------------------------------------*/
-static inline size_t OS_strnlen(const char *s, size_t maxlen)
-{
-    const char *end = (const char *)memchr(s, 0, maxlen);
-    if (end != NULL)
-    {
-        /* actual length of string is difference */
-        maxlen = end - s;
-    }
-    return maxlen;
-}
-
 #endif /* OS_SHARED_COMMON_H */
diff --git a/src/os/shared/src/osapi-common.c b/src/os/shared/src/osapi-common.c
@@ -423,3 +423,20 @@ void OS_ApplicationShutdown(uint8 flag)
      */
     OS_ApplicationShutdown_Impl();
 }
+
+/*----------------------------------------------------------------
+ *
+ *  Purpose: Implemented per public OSAL API
+ *           See description in API and header file for detail
+ *
+ *-----------------------------------------------------------------*/
+size_t OS_strnlen(const char *s, size_t maxlen)
+{
+    const char *end = (const char *)memchr(s, 0, maxlen);
+    if (end != NULL)
+    {
+        /* actual length of string is difference */
+        maxlen = end - s;
+    }
+    return maxlen;
+}
diff --git a/src/os/shared/src/osapi-condvar.c b/src/os/shared/src/osapi-condvar.c
@@ -66,6 +66,7 @@ OS_condvar_internal_record_t OS_condvar_table[OS_MAX_CONDVARS];
  *-----------------------------------------------------------------*/
 int32 OS_CondVarAPI_Init(void)
 {
+    // SAD: Using memset as sizeof(OS_condvar_table) ensures correct array size
     memset(OS_condvar_table, 0, sizeof(OS_condvar_table));
     return OS_SUCCESS;
 }
@@ -291,14 +292,15 @@ int32 OS_CondVarGetInfo(osal_id_t var_id, OS_condvar_prop_t *condvar_prop)
     /* Check parameters */
     OS_CHECK_POINTER(condvar_prop);
 
+    // SAD: Using memset as sizeof(OS_condvar_prop_t) ensures correct array size
     memset(condvar_prop, 0, sizeof(OS_condvar_prop_t));
 
     return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, OS_OBJECT_TYPE_OS_CONDVAR, var_id, &token);
     if (return_code == OS_SUCCESS)
     {
         record = OS_OBJECT_TABLE_GET(OS_global_condvar_table, token);
 
-        strncpy(condvar_prop->name, record->name_entry, sizeof(condvar_prop->name) - 1);
+        snprintf(condvar_prop->name, sizeof(condvar_prop->name), "%s", record->name_entry);
         condvar_prop->creator = record->creator;
 
         return_code = OS_CondVarGetInfo_Impl(&token, condvar_prop);

diff --git a/src/os/shared/src/osapi-errors.c b/src/os/shared/src/osapi-errors.c
@@ -110,6 +110,7 @@ char *OS_StatusToString(osal_status_t status, os_status_string_t *status_string)
 
     if (status_string != NULL)
     {
+        // SAD: No need to check snprintf return; OS_STATUS_STRING_LENGTH (12) is ample for all status values
         snprintf(*status_string, sizeof(*status_string), "%ld", OS_StatusToInteger(status));
         string = *status_string;
     }
@@ -149,7 +150,7 @@ int32 OS_GetErrorName(int32 error_num, os_err_name_t *err_name)
     {
         strncpy(*err_name, Error->Name, sizeof(*err_name) - 1);
         (*err_name)[sizeof(*err_name) - 1] = 0;
-        return_code                      = OS_SUCCESS;
+        return_code                        = OS_SUCCESS;
     }
     else
     {

diff --git a/src/unit-test-coverage/shared/src/coveragetest-common.c b/src/unit-test-coverage/shared/src/coveragetest-common.c
@@ -332,6 +332,28 @@ void Test_OS_NotifyEvent(void)
     OS_SharedGlobalVars.EventHandler = NULL;
 }
 
+void Test_OS_strnlen(void)
+{
+
+    size_t result;
+    char   str[OS_MAX_FILE_NAME];
+
+    memset(str, 0xFF, sizeof(str));
+
+    /* Test case where null character is not found */
+    result = OS_strnlen(str, sizeof(str));
+
+    UtAssert_INT32_EQ(result, sizeof(str));
+
+
+    /* Test case where null character is found */
+    str[OS_MAX_FILE_NAME - 1] = '\0';
+
+    result = OS_strnlen(str, sizeof(str));
+
+    UtAssert_INT32_EQ(result, sizeof(str) - 1);
+}
+
 /* ------------------- End of test cases --------------------------------------*/
 
 /* Osapi_Test_Setup
@@ -364,4 +386,5 @@ void UtTest_Setup(void)
     ADD_TEST(OS_ApplicationExit);
     ADD_TEST(OS_NotifyEvent);
     ADD_TEST(OS_API_Teardown);
+    ADD_TEST(OS_strnlen);
 }
diff --git a/src/unit-test-coverage/shared/src/coveragetest-filesys.c b/src/unit-test-coverage/shared/src/coveragetest-filesys.c
@@ -410,6 +410,7 @@ void Test_OS_TranslatePath(void)
      * int32 OS_TranslatePath(const char *VirtualPath, char *LocalPath)
      */
     char  LocalBuffer[OS_MAX_PATH_LEN];
+    char  DoubleBuffer[2 * OS_MAX_PATH_LEN];
     int32 expected = OS_SUCCESS;
     int32 actual   = ~OS_SUCCESS;
 
@@ -421,55 +422,98 @@ void Test_OS_TranslatePath(void)
     strcpy(OS_filesys_table[1].virtual_mountpt, "/cf");
     strcpy(OS_filesys_table[1].system_mountpt, "/mnt/cf");
 
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen("/cf/test"));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(OS_filesys_table[1].system_mountpt));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(OS_filesys_table[1].virtual_mountpt));
     actual = OS_TranslatePath("/cf/test", LocalBuffer);
     UtAssert_True(actual == expected, "OS_TranslatePath(/cf/test) (%ld) == OS_SUCCESS", (long)actual);
     UtAssert_True(strcmp(LocalBuffer, "/mnt/cf/test") == 0, "OS_TranslatePath(/cf/test) (%s)  == /mnt/cf/test",
                   LocalBuffer);
 
+    /* VirtPathLen >= OS_MAX_PATH_LEN */
+    memset(DoubleBuffer, 0xFF, sizeof(DoubleBuffer) - 1);
+    DoubleBuffer[sizeof(DoubleBuffer) - 1] = '\0';
+    UT_ResetState(UT_KEY(OS_strnlen));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(DoubleBuffer));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(OS_filesys_table[1].system_mountpt));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(OS_filesys_table[1].virtual_mountpt));
+    OSAPI_TEST_FUNCTION_RC(OS_TranslatePath(DoubleBuffer, LocalBuffer), OS_FS_ERR_PATH_TOO_LONG);
+
     /* Check various error paths */
     UtAssert_INT32_EQ(OS_TranslatePath("/cf/test", NULL), OS_INVALID_POINTER);
     UtAssert_INT32_EQ(OS_TranslatePath(NULL, LocalBuffer), OS_INVALID_POINTER);
 
     UT_SetDefaultReturnValue(UT_KEY(OCS_memchr), OS_ERROR);
     expected = OS_FS_ERR_PATH_TOO_LONG;
+    UT_ResetState(UT_KEY(OS_strnlen));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen("/cf/test"));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(OS_filesys_table[1].system_mountpt));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(OS_filesys_table[1].virtual_mountpt));
     actual   = OS_TranslatePath("/cf/test", LocalBuffer);
     UtAssert_True(actual == expected, "OS_TranslatePath() (%ld) == OS_FS_ERR_PATH_TOO_LONG", (long)actual);
-    UT_ClearDefaultReturnValue(UT_KEY(OCS_memchr));
 
     /* Invalid no '/' */
     expected = OS_FS_ERR_PATH_INVALID;
+    UT_ResetState(UT_KEY(OS_strnlen));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen("invalid/"));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(OS_filesys_table[1].system_mountpt));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(OS_filesys_table[1].virtual_mountpt));
     actual   = OS_TranslatePath("invalid", LocalBuffer);
     UtAssert_True(actual == expected, "OS_TranslatePath() (%ld) == OS_FS_ERR_PATH_INVALID", (long)actual);
 
     UT_SetDeferredRetcode(UT_KEY(OCS_memchr), 2, OS_ERROR);
     expected = OS_FS_ERR_NAME_TOO_LONG;
+    UT_ResetState(UT_KEY(OS_strnlen));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen("/cf/test"));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(OS_filesys_table[1].system_mountpt));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(OS_filesys_table[1].virtual_mountpt));
     actual   = OS_TranslatePath("/cf/test", LocalBuffer);
     UtAssert_True(actual == expected, "OS_TranslatePath(/cf/test) (%ld) == OS_FS_ERR_NAME_TOO_LONG", (long)actual);
 
     /* Invalid no leading '/' */
     expected = OS_FS_ERR_PATH_INVALID;
+    UT_ResetState(UT_KEY(OS_strnlen));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen("invalid/"));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(OS_filesys_table[1].system_mountpt));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(OS_filesys_table[1].virtual_mountpt));
     actual   = OS_TranslatePath("invalid/", LocalBuffer);
     UtAssert_True(actual == expected, "OS_TranslatePath() (%ld) == OS_FS_ERR_PATH_INVALID", (long)actual);
 
     UT_SetDefaultReturnValue(UT_KEY(OS_ObjectIdGetBySearch), OS_ERR_NAME_NOT_FOUND);
+    UT_ResetState(UT_KEY(OS_strnlen));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen("/cf/test"));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(OS_filesys_table[1].system_mountpt));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(OS_filesys_table[1].virtual_mountpt));
     actual = OS_TranslatePath("/cf/test", LocalBuffer);
     UtAssert_True(actual == expected, "OS_TranslatePath() (%ld) == OS_FS_ERR_PATH_INVALID", (long)actual);
     UT_ClearDefaultReturnValue(UT_KEY(OS_ObjectIdGetBySearch));
 
     /* VirtPathLen < VirtPathBegin */
     UT_SetDeferredRetcode(UT_KEY(OCS_memchr), 4, OS_ERROR);
     expected = OS_FS_ERR_PATH_INVALID;
+    UT_ResetState(UT_KEY(OS_strnlen));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen("/cf/test"));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(OS_filesys_table[1].system_mountpt));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(OS_filesys_table[1].virtual_mountpt));
     actual   = OS_TranslatePath("/cf/test", LocalBuffer);
     UtAssert_True(actual == expected, "OS_TranslatePath(/cf/test) (%ld) == OS_FS_ERR_PATH_INVALID", (long)actual);
 
     /* (SysMountPointLen + VirtPathLen) > OS_MAX_LOCAL_PATH_LEN */
     UT_SetDeferredRetcode(UT_KEY(OCS_memchr), 3, OS_ERROR);
     expected = OS_FS_ERR_PATH_TOO_LONG;
+    UT_ResetState(UT_KEY(OS_strnlen));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen("/cf/test"));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(OS_filesys_table[1].system_mountpt));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(OS_filesys_table[1].virtual_mountpt));
     actual   = OS_TranslatePath("/cf/test", LocalBuffer);
     UtAssert_True(actual == expected, "OS_TranslatePath(/cf/test) (%ld) == OS_FS_ERR_PATH_TOO_LONG", (long)actual);
 
     OS_filesys_table[1].flags = 0;
     expected                  = OS_ERR_INCORRECT_OBJ_STATE;
+    UT_ResetState(UT_KEY(OS_strnlen));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen("/cf/test"));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(OS_filesys_table[1].system_mountpt));
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(OS_filesys_table[1].virtual_mountpt));
     actual                    = OS_TranslatePath("/cf/test", LocalBuffer);
     UtAssert_True(actual == expected, "OS_TranslatePath(/cf/test) (%ld) == OS_ERR_INCORRECT_OBJ_STATE", (long)actual);
 }
@@ -497,39 +541,46 @@ void Test_OS_FileSys_FindVirtMountPoint(void)
     OS_filesys_table[1].flags              = 0;
     OS_filesys_table[1].virtual_mountpt[0] = 0;
 
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(OS_filesys_table[1].virtual_mountpt));
     result = OS_FileSys_FindVirtMountPoint((void *)refstr, &token, &refobj);
     UtAssert_True(!result, "OS_FileSys_FindVirtMountPoint(%s) (unmounted) == false", refstr);
 
     OS_filesys_table[1].flags = OS_FILESYS_FLAG_IS_MOUNTED_VIRTUAL;
 
     /* Branch coverage for mismatches */
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(OS_filesys_table[1].virtual_mountpt));
     result = OS_FileSys_FindVirtMountPoint((void *)refstr, &token, &refobj);
     UtAssert_True(!result, "OS_FileSys_FindVirtMountPoint(%s) (mountpt=%s) == false", refstr,
                   OS_filesys_table[1].virtual_mountpt);
 
     memset(OS_filesys_table[1].virtual_mountpt, 'a', sizeof(OS_filesys_table[1].virtual_mountpt));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(OS_filesys_table[1].virtual_mountpt));
     result = OS_FileSys_FindVirtMountPoint((void *)refstr, &token, &refobj);
     UtAssert_True(!result, "OS_FileSys_FindVirtMountPoint(%s) (mountpt=%s) == false", refstr,
                   OS_filesys_table[1].virtual_mountpt);
 
     /* Verify cases where one is a substring of the other -
      * these should also return false */
     strncpy(OS_filesys_table[1].virtual_mountpt, "/ut11", sizeof(OS_filesys_table[1].virtual_mountpt));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(OS_filesys_table[1].virtual_mountpt));
     result = OS_FileSys_FindVirtMountPoint((void *)refstr, &token, &refobj);
     UtAssert_True(!result, "OS_FileSys_FindVirtMountPoint(%s) (mountpt=%s) == false", refstr,
                   OS_filesys_table[1].virtual_mountpt);
 
     strncpy(OS_filesys_table[1].virtual_mountpt, "/u", sizeof(OS_filesys_table[1].virtual_mountpt));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(OS_filesys_table[1].virtual_mountpt));
     result = OS_FileSys_FindVirtMountPoint((void *)refstr, &token, &refobj);
     UtAssert_True(!result, "OS_FileSys_FindVirtMountPoint(%s) (mountpt=%s) == false", refstr,
                   OS_filesys_table[1].virtual_mountpt);
 
     strncpy(OS_filesys_table[1].virtual_mountpt, "/ut", sizeof(OS_filesys_table[1].virtual_mountpt));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(OS_filesys_table[1].virtual_mountpt));
     result = OS_FileSys_FindVirtMountPoint((void *)refstr, &token, &refobj);
     UtAssert_True(result, "OS_FileSys_FindVirtMountPoint(%s) (nominal) == true", refstr);
 
     /* Passing case with reference ending in "/" */
     strncpy(OS_filesys_table[1].virtual_mountpt, "/ut", sizeof(OS_filesys_table[1].virtual_mountpt));
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(OS_filesys_table[1].virtual_mountpt));
     result = OS_FileSys_FindVirtMountPoint((void *)refstr1, &token, &refobj);
     UtAssert_True(result, "OS_FileSys_FindVirtMountPoint(%s) (nominal) == true", refstr);
 }

diff --git a/src/unit-test-coverage/shared/src/coveragetest-idmap.c b/src/unit-test-coverage/shared/src/coveragetest-idmap.c
@@ -1087,6 +1087,7 @@ void Test_OS_GetResourceName(void)
     OSAPI_TEST_FUNCTION_RC(OS_GetResourceName(token.obj_id, NameBuffer, sizeof(NameBuffer)), OS_SUCCESS);
     UtAssert_True(strcmp(NameBuffer, "UTTask") == 0, "NameBuffer (%s) == UTTask", NameBuffer);
 
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(rptr->name_entry));
     OSAPI_TEST_FUNCTION_RC(OS_GetResourceName(token.obj_id, NameBuffer, OSAL_SIZE_C(2)), OS_ERR_NAME_TOO_LONG);
 
     /* Null entry */

diff --git a/src/ut-stubs/osapi-common-stubs.c b/src/ut-stubs/osapi-common-stubs.c
@@ -111,3 +111,20 @@ int32 OS_RegisterEventHandler(OS_EventHandler_t handler)
 
     return UT_GenStub_GetReturnValue(OS_RegisterEventHandler, int32);
 }
+
+/*
+ * ----------------------------------------------------
+ * Generated stub function for OS_strnlen()
+ * ----------------------------------------------------
+ */
+size_t OS_strnlen(const char *s, size_t maxlen)
+{
+    UT_GenStub_SetupReturnBuffer(OS_strnlen, size_t);
+
+    UT_GenStub_AddParam(OS_strnlen, const char *, s);
+    UT_GenStub_AddParam(OS_strnlen, size_t, maxlen);
+
+    UT_GenStub_Execute(OS_strnlen, Basic, NULL);
+
+    return UT_GenStub_GetReturnValue(OS_strnlen, size_t);
+}