Fix #863, check/report fcntl status

The fcntl() function is documented as returning -1 on error, so test for this condition and report errno if so. There is no recourse/handling - this just reports the error. However - failure to set the O_NONBLOCK flag via this method will fall back to blocking mode being used (timeouts will not work as a result).
nasa · Mar 15, 2021 · c11c9a6 · c11c9a6
1 parent ead5723
commit c11c9a6
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 8 deletions.
diff --git a/src/os/portable/os-impl-bsd-sockets.c b/src/os/portable/os-impl-bsd-sockets.c
@@ -156,10 +156,24 @@ int32 OS_SocketOpen_Impl(const OS_object_token_t *token)
      * any blocking would be done explicitly via the select() wrappers
      */
     os_flags = fcntl(impl->fd, F_GETFL);
-    os_flags |= OS_IMPL_SOCKET_FLAGS;
-    fcntl(impl->fd, F_SETFL, os_flags);
-
-    impl->selectable = ((os_flags & O_NONBLOCK) != 0);
+    if (os_flags == -1)
+    {
+        /* No recourse if F_GETFL fails - just report the error and move on. */
+        OS_DEBUG("fcntl(F_GETFL): %s\n", strerror(errno));
+    }
+    else
+    {
+        os_flags |= OS_IMPL_SOCKET_FLAGS;
+        if (fcntl(impl->fd, F_SETFL, os_flags) == -1)
+        {
+            /* No recourse if F_SETFL fails - just report the error and move on. */
+            OS_DEBUG("fcntl(F_SETFL): %s\n", strerror(errno));
+        }
+        else
+        {
+            impl->selectable = ((os_flags & O_NONBLOCK) != 0);
+        }
+    }
 
     return OS_SUCCESS;
 } /* end OS_SocketOpen_Impl */
@@ -360,10 +374,24 @@ int32 OS_SocketAccept_Impl(const OS_object_token_t *sock_token, const OS_object_
                  * any blocking would be done explicitly via the select() wrappers
                  */
                 os_flags = fcntl(conn_impl->fd, F_GETFL);
-                os_flags |= OS_IMPL_SOCKET_FLAGS;
-                fcntl(conn_impl->fd, F_SETFL, os_flags);
-
-                conn_impl->selectable = ((os_flags & O_NONBLOCK) != 0);
+                if (os_flags == -1)
+                {
+                    /* No recourse if F_GETFL fails - just report the error and move on. */
+                    OS_DEBUG("fcntl(F_GETFL): %s\n", strerror(errno));
+                }
+                else
+                {
+                    os_flags |= OS_IMPL_SOCKET_FLAGS;
+                    if (fcntl(conn_impl->fd, F_SETFL, os_flags) == -1)
+                    {
+                        /* No recourse if F_SETFL fails - just report the error and move on. */
+                        OS_DEBUG("fcntl(F_SETFL): %s\n", strerror(errno));
+                    }
+                    else
+                    {
+                        conn_impl->selectable = ((os_flags & O_NONBLOCK) != 0);
+                    }
+                }
             }
         }
     }

diff --git a/src/unit-test-coverage/portable/adaptors/inc/ut-adaptor-portable-posix-io.h b/src/unit-test-coverage/portable/adaptors/inc/ut-adaptor-portable-posix-io.h
@@ -39,5 +39,7 @@
  *
  *****************************************************/
 void UT_PortablePosixIOTest_Set_Selectable(osal_index_t local_id, bool is_selectable);
+bool UT_PortablePosixIOTest_Get_Selectable(osal_index_t local_id);
+void UT_PortablePosixIOTest_ResetImpl(osal_index_t local_id);
 
 #endif /* UT_ADAPTOR_PORTABLE_POSIX_IO_H  */
diff --git a/src/unit-test-coverage/portable/adaptors/src/ut-adaptor-portable-posix-io.c b/src/unit-test-coverage/portable/adaptors/src/ut-adaptor-portable-posix-io.c
@@ -31,7 +31,18 @@
 
 #include <os-impl-io.h>
 
+void UT_PortablePosixIOTest_ResetImpl(osal_index_t local_id)
+{
+    OS_impl_filehandle_table[local_id].fd         = -1;
+    OS_impl_filehandle_table[local_id].selectable = false;
+}
+
 void UT_PortablePosixIOTest_Set_Selectable(osal_index_t local_id, bool is_selectable)
 {
     OS_impl_filehandle_table[local_id].selectable = is_selectable;
 }
+
+bool UT_PortablePosixIOTest_Get_Selectable(osal_index_t local_id)
+{
+    return OS_impl_filehandle_table[local_id].selectable;
+}
diff --git a/src/unit-test-coverage/portable/src/coveragetest-bsd-sockets.c b/src/unit-test-coverage/portable/src/coveragetest-bsd-sockets.c
@@ -29,13 +29,17 @@
 #include "os-shared-file.h"
 
 #include "OCS_sys_socket.h"
+#include "OCS_fcntl.h"
+
+#include "ut-adaptor-portable-posix-io.h"
 
 void Test_OS_SocketOpen_Impl(void)
 {
     OS_object_token_t token = {0};
 
     /* Set up token for index 0 */
     token.obj_idx = UT_INDEX_0;
+    UT_PortablePosixIOTest_ResetImpl(token.obj_idx);
 
     /* Invalid socket type */
     OS_stream_table[0].socket_type = -1;
@@ -55,6 +59,25 @@ void Test_OS_SocketOpen_Impl(void)
     OS_stream_table[0].socket_type   = OS_SocketType_STREAM;
     OS_stream_table[0].socket_domain = OS_SocketDomain_INET6;
     OSAPI_TEST_FUNCTION_RC(OS_SocketOpen_Impl, (&token), OS_SUCCESS);
+    UtAssert_True(UT_PortablePosixIOTest_Get_Selectable(token.obj_idx), "Socket is selectable");
+
+    /* Failure in fcntl() GETFL */
+    UT_PortablePosixIOTest_ResetImpl(token.obj_idx);
+    UT_ResetState(UT_KEY(OCS_fcntl));
+    UT_SetDeferredRetcode(UT_KEY(OCS_fcntl), 1, -1);
+    OSAPI_TEST_FUNCTION_RC(OS_SocketOpen_Impl, (&token), OS_SUCCESS);
+    UtAssert_STUB_COUNT(OCS_fcntl, 1);
+    UtAssert_True(!UT_PortablePosixIOTest_Get_Selectable(token.obj_idx),
+                  "Socket not selectable without O_NONBLOCK flag");
+
+    /* Failure in fcntl() SETFL */
+    UT_PortablePosixIOTest_ResetImpl(token.obj_idx);
+    UT_ResetState(UT_KEY(OCS_fcntl));
+    UT_SetDeferredRetcode(UT_KEY(OCS_fcntl), 2, -1);
+    OSAPI_TEST_FUNCTION_RC(OS_SocketOpen_Impl, (&token), OS_SUCCESS);
+    UtAssert_STUB_COUNT(OCS_fcntl, 2);
+    UtAssert_True(!UT_PortablePosixIOTest_Get_Selectable(token.obj_idx),
+                  "Socket not selectable without O_NONBLOCK flag");
 }
 
 /* ------------------- End of test cases --------------------------------------*/