Merge pull request #927 from nasa/integration-candidate

osal Integration candidate: 2021-03-30

README.md

@@ -12,6 +12,17 @@ The autogenerated OSAL user's guide can be viewed at <https://github.com/nasa/cF
 ## Version History
 
 
+### Development Build: v5.1.0-rc1+dev367
+
+- Removes `SOFTWARE_BIG_BIT_ORDER` and `SOFTWARE_LITTLE_BIT_ORDER` macros from `common_types.h`. These are not needed by OSAL and cannot handle all cases.  Application code with endianness dependency that was relying on these symbols may break. Users should leverage code in cFE: `cfe_endian.h`. See <https://github.com/nasa/cFE/pull/1218> for more details.
+- Applies minor code and documentation cleanup: white space, typos, etc.
+- Adds test to get full coverage of vxworks in `os-impl-bsd-socket.c` resulting in full line coverage for OSAL
+- Adds more descriptive return codes if `OS_SymbolTableDump_Impl` does not do what is expected. Adds a new error `OS_ERR_OUTPUT_TOO_LARGE` if the size limit was insufficient. Return `OS_ERROR` if an empty file was written - this likely indicates some fundamental issue with the VxWorks symbol table. Returns `OS_ERR_NAME_TOO_LONG` if one of the symbol names was too long. Improves unit test to check for/verify these responses.
+- Removes the unneeded `OS_TaskRegister()` and all references to it in code, tests, and documentation. No impact to behavior, but does affect API and has depenedencies
+- Removes unused `-SCRIPT_MODE` flag in cmake
+- Remove comparison between `osal_id_t` and `integers` to use the provided comparison function, `OS_ObjectIdDefined()`. System builds and runs again when using a type-safe/non-integer osal_id_t type.
+- See <https://github.com/nasa/osal/pull/927>
+
 ### Development Build: v5.1.0-rc1+dev350
 
 - Moves copyblock size to a macro and add comments. Defines `OS_CP_BLOCK_SIZE` and adds clear documentation that it could be adjusted for page size, performance, etc.

src/examples/tasking-example/tasking-example.c

@@ -127,8 +127,6 @@ void task_1(void)
 
     OS_printf("Starting task 1\n");
 
-    OS_TaskRegister();
-
     while (1)
     {
         status = OS_MutSemTake(mutex_id);
@@ -171,8 +169,6 @@ void task_2(void)
 
     OS_printf("Starting task 2\n");
 
-    OS_TaskRegister();
-
     while (1)
     {
         status = OS_MutSemTake(mutex_id);
@@ -220,8 +216,6 @@ void task_3(void)
 
     OS_printf("Starting task 3\n");
 
-    OS_TaskRegister();
-
     while (1)
     {
         status = OS_QueueGet(msgq_id, (void *)&data_received, MSGQ_SIZE, &data_size, OS_PEND);

src/os/inc/common_types.h

@@ -138,34 +138,6 @@ extern "C"
     CompileTimeAssert(sizeof(int64) == 8, Typeint64WrongSize);
     CompileTimeAssert(sizeof(cpuaddr) >= sizeof(void *), TypePtrWrongSize);
 
-/*
- * TEMPORARY COMPATIBILITY MACRO
- *
- * Any code that depends on this macro should be fixed so as to not need it.
- * The value for this had been set by the BSP makefiles but this is not reliable,
- * especially on processors that support both big- and little- endian modes e.g.
- * ARM and MIPS.
- *
- * This is deprecated and only here to bridge the gap until code that depends
- * on this can be fixed.  Do not write any new code that uses this macro.
- *
- * If using an older makefile that defines one of the BIT_ORDER macros already,
- * then this entire section is skipped and the macro is used as-is.
- */
-#if !defined(SOFTWARE_BIG_BIT_ORDER) && !defined(SOFTWARE_LITTLE_BIT_ORDER)
-
-#if defined(__BYTE_ORDER) && __BYTE_ORDER == __BIG_ENDIAN || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || \
-    defined(__THUMBEB__) || defined(__AARCH64EB__) || defined(_MIBSEB) || defined(__MIBSEB) || defined(__MIBSEB__)
-/* It is a big-endian target architecture */
-#define SOFTWARE_BIG_BIT_ORDER
-#elif defined(__BYTE_ORDER) && __BYTE_ORDER == __LITTLE_ENDIAN || defined(__LITTLE_ENDIAN__) || defined(__ARMEL__) || \
-    defined(__THUMBEL__) || defined(__AARCH64EL__) || defined(_MIPSEL) || defined(__MIPSEL) || defined(__MIPSEL__)
-/* It is a little-endian target architecture */
-#define SOFTWARE_LITTLE_BIT_ORDER
-#endif
-
-#endif /* !defined(SOFTWARE_BIG_BIT_ORDER) && !defined(SOFTWARE_LITTLE_BIT_ORDER) */
-
 #ifdef __cplusplus
 }
 #endif

src/os/inc/osapi-dir.h

@@ -121,7 +121,7 @@ int32 OS_mkdir(const char *path, uint32 access);
 /**
  * @brief Removes a directory from the file system.
  *
- * Removes a directory from  the structure.
+ * Removes a directory from the structure.
  * The directory must be empty prior to this operation.
  *
  * @param[in]  path      The directory to remove

src/os/inc/osapi-error.h

@@ -84,6 +84,7 @@ typedef char os_err_name_t[OS_ERROR_NAME_LENGTH];
 #define OS_ERR_STREAM_DISCONNECTED     (-37) /**< @brief Stream disconnected */
 #define OS_ERR_OPERATION_NOT_SUPPORTED (-38) /**< @brief Requested operation not support on supplied object(s) */
 #define OS_ERR_INVALID_SIZE            (-40) /**< @brief Invalid Size */
+#define OS_ERR_OUTPUT_TOO_LARGE        (-41) /**< @brief Size of output exceeds limit  */
 
 /*
 ** Defines for File System Calls

src/os/inc/osapi-select.h

@@ -34,6 +34,10 @@
  * This is part of the select API and is manipulated using the
  * related API calls.  It should not be modified directly by applications.
  *
+ * Note: Math is to determine uint8 array size needed to represent
+ *       single bit OS_MAX_NUM_OPEN_FILES objects, + 7 rounds up
+ *       and 8 is the size of uint8.
+ *
  * @sa OS_SelectFdZero(), OS_SelectFdAdd(), OS_SelectFdClear(), OS_SelectFdIsSet()
  */
 typedef struct

src/os/inc/osapi-task.h

@@ -144,6 +144,7 @@ int32 OS_TaskInstallDeleteHandler(osal_task_entry function_pointer);
  * @brief Delay a task for specified amount of milliseconds
  *
  * Causes the current thread to be suspended from execution for the period of millisecond.
+ * This is a scheduled wait (clock_nanosleep/rtems_task_wake_after/taskDelay), not a "busy" wait.
  *
  * @param[in]   millisecond    Amount of time to delay
  *
@@ -169,18 +170,6 @@ int32 OS_TaskDelay(uint32 millisecond);
  */
 int32 OS_TaskSetPriority(osal_id_t task_id, osal_priority_t new_priority);
 
-/*-------------------------------------------------------------------------------------*/
-/**
- * @brief Obsolete
- * @deprecated Explicit registration call no longer needed
- *
- * Obsolete function retained for compatibility purposes.
- * Does Nothing in the current implementation.
- *
- * @return #OS_SUCCESS (always), see @ref OSReturnCodes
- */
-int32 OS_TaskRegister(void);
-
 /*-------------------------------------------------------------------------------------*/
 /**
  * @brief Obtain the task id of the calling task

src/os/inc/osapi-version.h

@@ -32,7 +32,7 @@
 /*
  * Development Build Macro Definitions
  */
-#define OS_BUILD_NUMBER   350
+#define OS_BUILD_NUMBER   367
 #define OS_BUILD_BASELINE "v5.1.0-rc1"
 
 /*

src/os/portable/os-impl-bsd-select.c

@@ -117,38 +117,39 @@ static int32 OS_FdSet_ConvertIn_Impl(int *os_maxfd, fd_set *os_set, const OS_FdS
     return status;
 } /* end OS_FdSet_ConvertIn_Impl */
 
-/*----------------------------------------------------------------
- * Function: OS_FdSet_ConvertOut_Impl
+/*----------------------------------------------------------------*/
+/**
+ * \brief Convert a POSIX fd_set structure into an OSAL OS_FdSet
+ *        which can then be returned back to the application.
  *
- *  Purpose: Local helper routine, not part of OSAL API.
+ * Local helper routine, not part of OSAL API.
  *
- *          Convert a POSIX fd_set structure into an OSAL OS_FdSet
- *          which can then be returned back to the application.
+ * This un-sets bits in OSAL_set that are set in the OS_set
  *
- *          This actually un-sets any bits in the "Input" parameter
- *          which are also set in the "output" parameter.
+ * \param[in]      OS_set   The fd_set from select
+ * \param[in, out] OSAL_set The OS_FdSet updated by this helper
  *-----------------------------------------------------------------*/
-static void OS_FdSet_ConvertOut_Impl(fd_set *output, OS_FdSet *Input)
+static void OS_FdSet_ConvertOut_Impl(fd_set *OS_set, OS_FdSet *OSAL_set)
 {
     size_t       offset;
     size_t       bit;
     osal_index_t id;
     uint8        objids;
     int          osfd;
 
-    for (offset = 0; offset < sizeof(Input->object_ids); ++offset)
+    for (offset = 0; offset < sizeof(OSAL_set->object_ids); ++offset)
     {
-        objids = Input->object_ids[offset];
+        objids = OSAL_set->object_ids[offset];
         bit    = 0;
         while (objids != 0)
         {
             id = OSAL_INDEX_C((offset * 8) + bit);
             if ((objids & 0x01) != 0 && id < OS_MAX_NUM_OPEN_FILES)
             {
                 osfd = OS_impl_filehandle_table[id].fd;
-                if (osfd < 0 || !FD_ISSET(osfd, output))
+                if (osfd < 0 || !FD_ISSET(osfd, OS_set))
                 {
-                    Input->object_ids[offset] &= ~(1 << bit);
+                    OSAL_set->object_ids[offset] &= ~(1 << bit);
                 }
             }
             ++bit;
@@ -187,7 +188,7 @@ static int32 OS_DoSelect(int maxfd, fd_set *rd_set, fd_set *wr_set, int32 msecs)
     }
     else
     {
-        /* eliminates a false warning about possibly uninitialized use */
+        /* Zero for consistency and to avoid possible confusion if not cleared */
         memset(&ts_end, 0, sizeof(ts_end));
     }
 

src/os/portable/os-impl-posix-dirs.c

@@ -82,7 +82,7 @@ int32 OS_DirCreate_Impl(const char *local_path, uint32 access)
 
         if (errno == EEXIST)
         {
-            /* it exists, but not necessarily a directory */
+            /* Success if already exists and is a directory */
             if (stat(local_path, &st) == 0 && S_ISDIR(st.st_mode))
             {
                 return_code = OS_SUCCESS;

src/os/shared/inc/os-shared-task.h

@@ -166,7 +166,7 @@ int32 OS_TaskGetInfo_Impl(const OS_object_token_t *token, OS_task_prop_t *task_p
     Purpose: Perform registration actions after new task creation
 
         NOTE: This is invoked via the OS_TaskEntryPoint() immediately
-              after new task creation, not through OS_TaskRegister() API
+              after new task creation
 
     Returns: OS_SUCCESS on success, or relevant error code
  ------------------------------------------------------------------*/

src/os/shared/src/osapi-common.c

@@ -255,21 +255,11 @@ void OS_ApplicationExit(int32 Status)
     }
 } /* end OS_ApplicationExit */
 
-/*---------------------------------------------------------------------------------------
-   Name: OS_CleanUpObject
-
-   Purpose: Implements a single API call that can delete ANY object
-            Will dispatch to the correct delete implementation for that object type
-
-   Returns: None
-
----------------------------------------------------------------------------------------*/
-
 /*----------------------------------------------------------------
  *
  * Function: OS_CleanUpObject
  *
- *  Purpose: Local helper routine, not part of OSAL API.
+ *  Purpose: Local helper routine that can delete ANY object, not part of OSAL API.
  *
  *-----------------------------------------------------------------*/
 void OS_CleanUpObject(osal_id_t object_id, void *arg)

src/os/shared/src/osapi-idmap.c

@@ -971,7 +971,7 @@ int32 OS_ObjectIdGetBySearch(OS_lock_mode_t lock_mode, osal_objtype_t idtype, OS
         /*
          * The "ConvertToken" routine will return with the global lock
          * in a state appropriate for returning to the caller, as indicated
-         * by the "check_mode" parameter.
+         * by the "lock_mode" parameter.
          */
         return_code = OS_ObjectIdConvertToken(token);
     }

src/os/shared/src/osapi-printf.c

@@ -281,10 +281,7 @@ void OS_printf(const char *String, ...)
     }
     else if (OS_SharedGlobalVars.PrintfEnabled)
     {
-        /*
-         * Call vsnprintf() to determine the actual size of the
-         * string we are going to write to the buffer after formatting.
-         */
+        /* Format and determine the size of string to write */
         va_start(va, String);
         actualsz = vsnprintf(msg_buffer, sizeof(msg_buffer), String, va);
         va_end(va);

src/os/shared/src/osapi-queue.c

@@ -87,7 +87,7 @@ int32 OS_QueueAPI_Init(void)
  *           See description in API and header file for detail
  *
  *-----------------------------------------------------------------*/
-int32 OS_QueueCreate(osal_id_t *queue_id, const char *queue_name, osal_blockcount_t queue_depth, size_t data_size,
+int32 OS_QueueCreate(osal_id_t *queue_id, const char *queue_name, osal_blockcount_t queue_depth, size_t max_size,
                      uint32 flags)
 {
     int32                       return_code;
@@ -97,7 +97,7 @@ int32 OS_QueueCreate(osal_id_t *queue_id, const char *queue_name, osal_blockcoun
     /* validate inputs */
     OS_CHECK_POINTER(queue_id);
     OS_CHECK_APINAME(queue_name);
-    OS_CHECK_SIZE(data_size);
+    OS_CHECK_SIZE(max_size);
     ARGCHECK(queue_depth <= OS_QUEUE_MAX_DEPTH, OS_QUEUE_INVALID_SIZE);
 
     /* Note - the common ObjectIdAllocate routine will lock the object type and leave it locked. */
@@ -110,7 +110,7 @@ int32 OS_QueueCreate(osal_id_t *queue_id, const char *queue_name, osal_blockcoun
         OS_OBJECT_INIT(token, queue, queue_name, queue_name);
 
         queue->max_depth = queue_depth;
-        queue->max_size  = data_size;
+        queue->max_size  = max_size;
 
         /* Now call the OS-specific implementation.  This reads info from the queue table. */
         return_code = OS_QueueCreate_Impl(&token, flags);

src/os/shared/src/osapi-select.c

@@ -141,6 +141,11 @@ int32 OS_SelectFdAdd(OS_FdSet *Set, osal_id_t objid)
     return_code = OS_ObjectIdToArrayIndex(OS_OBJECT_TYPE_OS_STREAM, objid, &local_id);
     if (return_code == OS_SUCCESS)
     {
+        /*
+         * Sets the bit in the uint8 object_ids array that corresponds
+         * to the local_id where local_id >> 3 determines the array element,
+         * and the mask/shift sets the bit within that element.
+         */
         Set->object_ids[local_id >> 3] |= 1 << (local_id & 0x7);
     }
 
@@ -166,6 +171,11 @@ int32 OS_SelectFdClear(OS_FdSet *Set, osal_id_t objid)
     return_code = OS_ObjectIdToArrayIndex(OS_OBJECT_TYPE_OS_STREAM, objid, &local_id);
     if (return_code == OS_SUCCESS)
     {
+        /*
+         * Clears the bit in the uint8 object_ids array that corresponds
+         * to the local_id where local_id >> 3 determines the array element,
+         * and the mask/shift clears the bit within that element.
+         */
         Set->object_ids[local_id >> 3] &= ~(1 << (local_id & 0x7));
     }
 
@@ -194,5 +204,10 @@ bool OS_SelectFdIsSet(OS_FdSet *Set, osal_id_t objid)
         return false;
     }
 
+    /*
+     * Returns boolean for if the bit in the uint8 object_ids array that corresponds
+     * to the local_id is set where local_id >> 3 determines the array element,
+     * and the mask/shift checks the bit within that element.
+     */
     return ((Set->object_ids[local_id >> 3] >> (local_id & 0x7)) & 0x1);
 } /* end OS_SelectFdIsSet */

src/os/shared/src/osapi-task.c

@@ -329,25 +329,6 @@ int32 OS_TaskSetPriority(osal_id_t task_id, osal_priority_t new_priority)
     return return_code;
 } /* end OS_TaskSetPriority */
 
-/*----------------------------------------------------------------
- *
- * Function: OS_TaskRegister
- *
- *  Purpose: Implemented per public OSAL API
- *           See description in API and header file for detail
- *
- *-----------------------------------------------------------------*/
-int32 OS_TaskRegister(void)
-{
-    OS_object_token_t token;
-
-    /*
-     * Just to retain compatibility (really, only the unit test cares)
-     * this will return NON success when called from a non-task context
-     */
-    return OS_ObjectIdGetById(OS_LOCK_MODE_NONE, LOCAL_OBJID_TYPE, OS_TaskGetId_Impl(), &token);
-} /* end OS_TaskRegister */
-
 /*----------------------------------------------------------------
  *
  * Function: OS_TaskGetId

src/os/vxworks/src/os-impl-console.c

@@ -119,6 +119,7 @@ int OS_VxWorks_ConsoleTask_Entry(int arg)
         OS_ObjectIdRelease(&token);
     }
 
+    /* Return OK since called from taskSpawn, error is reported in debug message */
     return OK;
 } /* end OS_ConsoleTask_Entry */
 

src/os/vxworks/src/os-impl-symtab.c

@@ -175,7 +175,7 @@ BOOL OS_SymTableIterator_Impl(char *name, SYM_VALUE val, SYM_TYPE type, _Vx_usr_
     if (memchr(name, 0, OS_MAX_SYM_LEN) == NULL)
     {
         OS_DEBUG("%s(): symbol name too long\n", __func__);
-        state->StatusCode = OS_ERROR;
+        state->StatusCode = OS_ERR_NAME_TOO_LONG;
         return (false);
     }
 
@@ -190,6 +190,7 @@ BOOL OS_SymTableIterator_Impl(char *name, SYM_VALUE val, SYM_TYPE type, _Vx_usr_
         ** However this is not considered an error, just a stop condition.
         */
         OS_DEBUG("%s(): symbol table size exceeded\n", __func__);
+        state->StatusCode = OS_ERR_OUTPUT_TOO_LARGE;
         return (false);
     }
 
@@ -264,6 +265,16 @@ int32 OS_SymbolTableDump_Impl(const char *filename, size_t size_limit)
         close(state->fd);
     }
 
+    /*
+     * If output size was zero this means a failure of the symEach call,
+     * in that it didn't iterate over anything at all.
+     */
+    if (state->StatusCode == OS_SUCCESS && state->CurrSize == 0)
+    {
+        OS_DEBUG("%s(): No symbols found!\n", __func__);
+        state->StatusCode = OS_ERROR;
+    }
+
     return (state->StatusCode);
 
 } /* end OS_SymbolTableDump_Impl */