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

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;
 }

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

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

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

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

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
     {

src/unit-test-coverage/shared/src/coveragetest-common.c

@@ -332,6 +332,27 @@ 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 +385,5 @@ void UtTest_Setup(void)
     ADD_TEST(OS_ApplicationExit);
     ADD_TEST(OS_NotifyEvent);
     ADD_TEST(OS_API_Teardown);
+    ADD_TEST(OS_strnlen);
 }

src/unit-test-coverage/shared/src/coveragetest-filesys.c

@@ -430,18 +430,18 @@ void Test_OS_TranslatePath(void)
     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;
-    actual   = OS_TranslatePath("/cf/test", LocalBuffer);
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 1, OS_MAX_PATH_LEN + 1);
+    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));
+    UT_ResetState(UT_KEY(OS_strnlen));
 
     /* Invalid no '/' */
     expected = OS_FS_ERR_PATH_INVALID;
     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);
+    UT_SetDeferredRetcode(UT_KEY(OCS_memchr), 1, OS_ERROR);
     expected = OS_FS_ERR_NAME_TOO_LONG;
     actual   = OS_TranslatePath("/cf/test", LocalBuffer);
     UtAssert_True(actual == expected, "OS_TranslatePath(/cf/test) (%ld) == OS_FS_ERR_NAME_TOO_LONG", (long)actual);
@@ -457,13 +457,13 @@ void Test_OS_TranslatePath(void)
     UT_ClearDefaultReturnValue(UT_KEY(OS_ObjectIdGetBySearch));
 
     /* VirtPathLen < VirtPathBegin */
-    UT_SetDeferredRetcode(UT_KEY(OCS_memchr), 4, OS_ERROR);
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 1, 1);
     expected = OS_FS_ERR_PATH_INVALID;
     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);
+    UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, OS_MAX_LOCAL_PATH_LEN + 1);
     expected = OS_FS_ERR_PATH_TOO_LONG;
     actual   = OS_TranslatePath("/cf/test", LocalBuffer);
     UtAssert_True(actual == expected, "OS_TranslatePath(/cf/test) (%ld) == OS_FS_ERR_PATH_TOO_LONG", (long)actual);

src/ut-stubs/CMakeLists.txt

@@ -60,6 +60,7 @@ add_library(ut_osapi_stubs STATIC
     osapi-clock-stubs.c
     osapi-clock-handlers.c
     osapi-common-stubs.c
+    osapi-common-handlers.c
     osapi-condvar-stubs.c
     osapi-countsem-stubs.c
     osapi-countsem-handlers.c

src/ut-stubs/osapi-common-handlers.c

@@ -0,0 +1,72 @@
+/************************************************************************
+ * NASA Docket No. GSC-18,719-1, and identified as “core Flight System: Bootes”
+ *
+ * Copyright (c) 2020 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
+ *
+ *
+ * Stub implementations for the functions defined in the OSAL API
+ *
+ * The stub implementation can be used for unit testing applications built
+ * on top of OSAL.  The stubs do not do any real function, but allow
+ * the return code to be crafted such that error paths in the application
+ * can be executed.
+ */
+
+#include "osapi-common.h" /* OSAL public API for this subsystem */
+#include "utstub-helpers.h"
+
+/*
+ * -----------------------------------------------------------------
+ * Default handler implementation for 'OS_strnlen' stub
+ * -----------------------------------------------------------------
+ */
+void UT_DefaultHandler_OS_strnlen(void *UserObj, UT_EntryKey_t FuncKey, const UT_StubContext_t *Context)
+{
+    const char *s;
+    const char *end;
+    size_t      maxlen;
+    size_t      retval;
+    int32       status;
+
+    if (UT_Stub_GetInt32StatusCode(Context, &status))
+    {
+        /* If a retval was supplied in the test case, then use it */
+        retval = status;
+    }
+    else
+    {
+        s      = UT_Hook_GetArgValueByName(Context, "s", const char *);
+        maxlen = UT_Hook_GetArgValueByName(Context, "maxlen", size_t);
+
+        /* This is basically the real impl of strnlen, as it
+         * usually needs to give back the appropriate value in
+         * order to follow the expected path */
+        end = memchr(s, 0, maxlen);
+        if (end == NULL)
+        {
+            retval = maxlen;
+        }
+        else
+        {
+            retval = end - s;
+        }
+    }
+
+    UT_Stub_SetReturnValue(FuncKey, retval);
+}

src/ut-stubs/osapi-common-stubs.c

@@ -25,6 +25,8 @@
 #include "osapi-common.h"
 #include "utgenstub.h"
 
+void UT_DefaultHandler_OS_strnlen(void *, UT_EntryKey_t, const UT_StubContext_t *);
+
 /*
  * ----------------------------------------------------
  * Generated stub function for OS_API_Init()
@@ -111,3 +113,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, UT_DefaultHandler_OS_strnlen);
+
+    return UT_GenStub_GetReturnValue(OS_strnlen, size_t);
+}