Merge pull request #917 from nasa/integration-candidate

osal Integration Candidate: 2021-03-23

Author: astrogeco

CONTRIBUTING.md

@@ -0,0 +1,3 @@
+# Contributing Guide
+
+Please see our [top-level contributing guide](https://github.com/nasa/cFS/blob/main/CONTRIBUTING.md) for more information on how to contribute. 

README.md

@@ -11,6 +11,35 @@ The autogenerated OSAL user's guide can be viewed at <https://github.com/nasa/cF
 
 ## Version History
 
+
+### Development Build: v5.1.0-rc1+dev347
+
+- 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.
+- Removes while loop
+- Replaces all #includes of <os and <OSC_ matches with " to match coding standard.
+- Consolidates the duplicated switch in `OS_SocketOpen_Impl`
+- Add const to input pointers for `OS_FdSet_ConvertIn_Impl` and `OS_ObjectIdTransactionFinish`
+- Removes network prototypes defined in `osapi_sockets.h` that are also in `osapi_network.h`
+- Removes `NULL` redefine from `common_types.h`
+- Adds `Contributing.md` that points to bundle-level contribution guide
+- Reports test cases that "fail" as "not implemented" with new `UtAssert_NA` macro instead of `UtPrintf`
+- Calls to `OS_SelectSingle` and `OS_SelectMultiple` will fail if an FD within the set is outside the range of the underlying `FD_SETSIZE` from the C library.
+- Fixes calculation used for the relative time interval in the `select()` call. Also adds a UT case that specifically exercises the carryover described. Fixes delay when this carry condition is hit
+- Documents algorithm that provides application-controlled timeout on the connection initiation. Also adds a debug statement if the connect fails for a reason other than `EINPROGRESS`. No impact to normal behavior.
+- Adds check for `EAGAIN` if the system fails to allocate kernel-internal resources.
+- Adds a `CompileTimeAssert` to confirm that the size of the abstract buffer for socket addresses is large enough to store any of the enabled address types thereby removing the need for runtime tests.
+- With this change, if `OS_SOCKADDR_MAX_LENis` not large enough for the address type, osal will fail to compile. This enforces that the abstract size is large enough for any/all enabled address types, regardless of what is actually used.
+- Adds missing functional test for `OS_ShellOutputToFile`
+- Add test for `fcntl()` error return of -1 and report errno. If setting `O_NONBLOCK` fails, then debug message is printed and blocking mode is used and timeouts will not work as a result.
+- Improves error codes when attempting to seek on a pipe/socket. Translates the `OS_ERR_OPERATION_NOT_SUPPORTED` error rather than "not implemented". The `ESPIPE` errno means that seeking is not supported on the given file handle.
+- Renames `OS_U32ValueWrapper_t` as `OS_VoidPtrValueWrapper_t` to better indicate its purpose. The point is to pass a value through a `void*`. Adds a compile-time assert to check that this is only used to directly pass values which have a size of less than or equal to sizeof(void*).
+- Refactors the return statement for `OS_FileSys_FindVirtMountPoint()` so it is easier to read and adds some informational comments.
+- Reports an error if calling `timer_gettime` after `timer_settime` fails.
+- Returns `OS_ERROR` status to caller after an error on moduleInfoGet()
+- Removes an extraneous/unreachable OS_ObjectIdDefined check and its accompanying debug statement. The only way this check could have been reached would be if the normal unlock process was bypassed such that the underlying OS mutex was unlocked but OSAL state still had it owned by a task. This condition never happens at runtime.
+- Updates documentation for `OS_MAX_MODULE`
+- See <https://github.com/nasa/osal/pull/917>
+
 ### Development Build: v5.1.0-rc1+dev297
 
 - Fix #836, Add Testing Tools to the Security Policy

src/os/inc/common_types.h

@@ -125,10 +125,6 @@ extern "C"
      */
     typedef void (*OS_ArgCallback_t)(osal_id_t object_id, void *arg);
 
-#ifndef NULL /* pointer to nothing */
-#define NULL ((void *)0)
-#endif
-
     /*
     ** Check Sizes
     */

src/os/inc/osapi-sockets.h

@@ -368,35 +368,6 @@ int32 OS_SocketGetIdByName(osal_id_t *sock_id, const char *sock_name);
  */
 int32 OS_SocketGetInfo(osal_id_t sock_id, OS_socket_prop_t *sock_prop);
 
-/*-------------------------------------------------------------------------------------*/
-/**
- * @brief  Gets the network ID of the local machine
- *
- * The ID is an implementation-defined value and may not be consistent
- * in meaning across different platform types.
- *
- * @note This API may be removed in a future version of OSAL due to
- *       inconsistencies between platforms.
- *
- * @return The ID or fixed value of -1 if the host id could not be found.
- *         Note it is not possible to differentiate between error codes and valid
- *         network IDs here. It is assumed, however, that -1 is never a valid ID.
- */
-int32 OS_NetworkGetID(void);
-
-/*-------------------------------------------------------------------------------------*/
-/**
- * @brief Gets the local machine network host name
- *
- * If configured in the underlying network stack,
- * this function retrieves the local hostname of the system.
- *
- * @param[out]  host_name    Buffer to hold name information
- * @param[in]   name_len     Maximum length of host name buffer
- *
- * @return Execution status, see @ref OSReturnCodes
- */
-int32 OS_NetworkGetHostName(char *host_name, size_t name_len);
 /**@}*/
 
 #endif

src/os/inc/osapi-version.h

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

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

@@ -72,32 +72,40 @@
  *
  * returns: Highest numbered file descriptor in the output fd_set
  *-----------------------------------------------------------------*/
-static int OS_FdSet_ConvertIn_Impl(fd_set *os_set, OS_FdSet *OSAL_set)
+static int32 OS_FdSet_ConvertIn_Impl(int *os_maxfd, fd_set *os_set, const OS_FdSet *OSAL_set)
 {
     size_t       offset;
     size_t       bit;
     osal_index_t id;
     uint8        objids;
     int          osfd;
-    int          maxfd;
+    int32        status;
 
-    maxfd = -1;
+    status = OS_SUCCESS;
     for (offset = 0; offset < sizeof(OSAL_set->object_ids); ++offset)
     {
         objids = OSAL_set->object_ids[offset];
         bit    = 0;
         while (objids != 0)
         {
-            if (objids & 0x01)
+            id = OSAL_INDEX_C((offset * 8) + bit);
+            if ((objids & 0x01) != 0 && id < OS_MAX_NUM_OPEN_FILES)
             {
-                id   = OSAL_INDEX_C((offset * 8) + bit);
                 osfd = OS_impl_filehandle_table[id].fd;
-                if (osfd >= 0 && OS_impl_filehandle_table[id].selectable)
+                if (osfd >= 0)
                 {
-                    FD_SET(osfd, os_set);
-                    if (osfd > maxfd)
+                    if (osfd >= FD_SETSIZE || !OS_impl_filehandle_table[id].selectable)
+                    {
+                        /* out of range of select() implementation */
+                        status = OS_ERR_OPERATION_NOT_SUPPORTED;
+                    }
+                    else
                     {
-                        maxfd = osfd;
+                        FD_SET(osfd, os_set);
+                        if (osfd > *os_maxfd)
+                        {
+                            *os_maxfd = osfd;
+                        }
                     }
                 }
             }
@@ -106,7 +114,7 @@ static int OS_FdSet_ConvertIn_Impl(fd_set *os_set, OS_FdSet *OSAL_set)
         }
     }
 
-    return maxfd;
+    return status;
 } /* end OS_FdSet_ConvertIn_Impl */
 
 /*----------------------------------------------------------------
@@ -134,9 +142,9 @@ static void OS_FdSet_ConvertOut_Impl(fd_set *output, OS_FdSet *Input)
         bit    = 0;
         while (objids != 0)
         {
-            if (objids & 0x01)
+            id = OSAL_INDEX_C((offset * 8) + bit);
+            if ((objids & 0x01) != 0 && id < OS_MAX_NUM_OPEN_FILES)
             {
-                id   = OSAL_INDEX_C((offset * 8) + bit);
                 osfd = OS_impl_filehandle_table[id].fd;
                 if (osfd < 0 || !FD_ISSET(osfd, output))
                 {
@@ -214,12 +222,12 @@ static int32 OS_DoSelect(int maxfd, fd_set *rd_set, fd_set *wr_set, int32 msecs)
             if (tv.tv_usec < 0)
             {
                 tv.tv_usec += 1000000;
-                ++tv.tv_sec;
+                --tv.tv_sec;
             }
         }
 
         os_status = select(maxfd + 1, rd_set, wr_set, NULL, tvptr);
-    } while (os_status < 0 && errno == EINTR);
+    } while (os_status < 0 && (errno == EINTR || errno == EAGAIN));
 
     if (os_status < 0)
     {
@@ -267,6 +275,12 @@ int32 OS_SelectSingle_Impl(const OS_object_token_t *token, uint32 *SelectFlags,
         return OS_ERR_OPERATION_NOT_SUPPORTED;
     }
 
+    if (impl->fd >= FD_SETSIZE)
+    {
+        /* out of range of select() implementation */
+        return OS_ERR_OPERATION_NOT_SUPPORTED;
+    }
+
     if (*SelectFlags != 0)
     {
         FD_ZERO(&wr_set);
@@ -319,41 +333,41 @@ int32 OS_SelectMultiple_Impl(OS_FdSet *ReadSet, OS_FdSet *WriteSet, int32 msecs)
 {
     fd_set wr_set;
     fd_set rd_set;
-    int    osfd;
     int    maxfd;
     int32  return_code;
 
-    /*
-     * This return code will be used if the set(s) do not
-     * contain any file handles capable of select().  It
-     * will be overwritten with the real result of the
-     * select call, if selectable file handles were specified.
-     */
-    return_code = OS_ERR_OPERATION_NOT_SUPPORTED;
     FD_ZERO(&rd_set);
     FD_ZERO(&wr_set);
     maxfd = -1;
     if (ReadSet != NULL)
     {
-        osfd = OS_FdSet_ConvertIn_Impl(&rd_set, ReadSet);
-        if (osfd > maxfd)
+        return_code = OS_FdSet_ConvertIn_Impl(&maxfd, &rd_set, ReadSet);
+        if (return_code != OS_SUCCESS)
         {
-            maxfd = osfd;
+            return return_code;
         }
     }
     if (WriteSet != NULL)
     {
-        osfd = OS_FdSet_ConvertIn_Impl(&wr_set, WriteSet);
-        if (osfd > maxfd)
+        return_code = OS_FdSet_ConvertIn_Impl(&maxfd, &wr_set, WriteSet);
+        if (return_code != OS_SUCCESS)
         {
-            maxfd = osfd;
+            return return_code;
         }
     }
 
     if (maxfd >= 0)
     {
         return_code = OS_DoSelect(maxfd, &rd_set, &wr_set, msecs);
     }
+    else
+    {
+        /*
+         * This return code will be used if the set(s) were
+         * both empty/NULL or otherwise did not contain valid filehandles.
+         */
+        return_code = OS_ERR_INVALID_ID;
+    }
 
     if (return_code == OS_SUCCESS)
     {

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

@@ -73,6 +73,15 @@ typedef union
 #endif
 } OS_SockAddr_Accessor_t;
 
+/*
+ * Confirm that the abstract socket address buffer size (OS_SOCKADDR_MAX_LEN) is
+ * large enough to store any of the enabled address types.  If this is true, the
+ * size of the above union will match OS_SOCKADDR_MAX_LEN.  However, if any
+ * implemention-provided struct types are larger than this, the union will be
+ * larger, and this indicates a configuration error.
+ */
+CompileTimeAssert(sizeof(OS_SockAddr_Accessor_t) == OS_SOCKADDR_MAX_LEN, SockAddrSize);
+
 /****************************************************************************************
                                     Sockets API
  ***************************************************************************************/
@@ -102,10 +111,13 @@ int32 OS_SocketOpen_Impl(const OS_object_token_t *token)
     switch (stream->socket_type)
     {
         case OS_SocketType_DATAGRAM:
-            os_type = SOCK_DGRAM;
+            os_type  = SOCK_DGRAM;
+            os_proto = IPPROTO_UDP;
             break;
+
         case OS_SocketType_STREAM:
-            os_type = SOCK_STREAM;
+            os_type  = SOCK_STREAM;
+            os_proto = IPPROTO_TCP;
             break;
 
         default:
@@ -126,17 +138,6 @@ int32 OS_SocketOpen_Impl(const OS_object_token_t *token)
             return OS_ERR_NOT_IMPLEMENTED;
     }
 
-    /* Only AF_INET* at this point, can add cases if support is expanded */
-    switch (stream->socket_type)
-    {
-        case OS_SocketType_DATAGRAM:
-            os_proto = IPPROTO_UDP;
-            break;
-        case OS_SocketType_STREAM:
-            os_proto = IPPROTO_TCP;
-            break;
-    }
-
     impl->fd = socket(os_domain, os_type, os_proto);
     if (impl->fd < 0)
     {
@@ -200,7 +201,7 @@ int32 OS_SocketBind_Impl(const OS_object_token_t *token, const OS_SockAddr_t *Ad
             break;
     }
 
-    if (addrlen == 0 || addrlen > OS_SOCKADDR_MAX_LEN)
+    if (addrlen == 0)
     {
         return OS_ERR_BAD_ADDRESS;
     }
@@ -273,10 +274,24 @@ int32 OS_SocketConnect_Impl(const OS_object_token_t *token, const OS_SockAddr_t
         {
             if (errno != EINPROGRESS)
             {
+                OS_DEBUG("connect: %s\n", strerror(errno));
                 return_code = OS_ERROR;
             }
             else
             {
+                /*
+                 * If the socket was created in nonblocking mode (O_NONBLOCK flag) then the connect
+                 * runs in the background and connect() returns EINPROGRESS.  In this case we still
+                 * want to provide the "normal" (blocking) semantics to the calling app, such that
+                 * when OS_SocketConnect() returns, the socket is ready for use.
+                 *
+                 * To provide consistent behavior to calling apps, this does a select() to wait
+                 * for the socket to become writable, meaning that the remote side is connected.
+                 *
+                 * An important point here is that the calling app can control the timeout.  If the
+                 * normal/blocking connect() was used, the OS/IP stack controls the timeout, and it
+                 * can be quite long.
+                 */
                 operation = OS_STREAM_STATE_WRITABLE;
                 if (impl->selectable)
                 {
@@ -290,6 +305,10 @@ int32 OS_SocketConnect_Impl(const OS_object_token_t *token, const OS_SockAddr_t
                     }
                     else
                     {
+                        /*
+                         * The SO_ERROR socket flag should also read back zero.
+                         * If not zero, something went wrong during connect
+                         */
                         sockopt   = 0;
                         slen      = sizeof(sockopt);
                         os_status = getsockopt(impl->fd, SOL_SOCKET, SO_ERROR, &sockopt, &slen);
@@ -560,7 +579,7 @@ int32 OS_SocketAddrInit_Impl(OS_SockAddr_t *Addr, OS_SocketDomain_t Domain)
             break;
     }
 
-    if (addrlen == 0 || addrlen > OS_SOCKADDR_MAX_LEN)
+    if (addrlen == 0)
     {
         return OS_ERR_NOT_IMPLEMENTED;
     }

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

@@ -138,7 +138,7 @@ int32 OS_GenericSeek_Impl(const OS_object_token_t *token, int32 offset, uint32 w
              * Use a different error code to differentiate from an
              * error involving a bad whence/offset
              */
-            retval = OS_ERR_NOT_IMPLEMENTED;
+            retval = OS_ERR_OPERATION_NOT_SUPPORTED;
         }
         else
         {

src/os/posix/inc/os-impl-tasks.h

@@ -28,7 +28,7 @@
 #ifndef OS_IMPL_TASKS_H
 #define OS_IMPL_TASKS_H
 
-#include <os-shared-task.h>
+#include "os-shared-task.h"
 
 #include "osconfig.h"
 #include <pthread.h>

src/os/posix/inc/os-posix.h

@@ -53,7 +53,7 @@
 /*
  * Use the global definitions from the shared layer
  */
-#include <os-shared-globaldefs.h>
+#include "os-shared-globaldefs.h"
 
 /****************************************************************************************
                                      DEFINES