From 91b3921e2a638962e46e2c58e756168be54722cd Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Wed, 13 May 2020 19:41:27 -0400 Subject: [PATCH] Fix #453, Select API and unit test updates Confirm that the "selectable" flag is set before calling the underlying select() API. Also update unit tests to match. --- src/os/inc/osapi.h | 1 + src/os/portable/os-impl-bsd-select.c | 23 +++++++++- src/os/shared/inc/os-shared-select.h | 2 + .../portable/src/coveragetest-bsd-select.c | 4 ++ .../oscore-test/ut_oscore_select_test.c | 44 +++++++++++-------- 5 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/os/inc/osapi.h b/src/os/inc/osapi.h index c34b614be..070950a04 100644 --- a/src/os/inc/osapi.h +++ b/src/os/inc/osapi.h @@ -77,6 +77,7 @@ #define OS_ERR_INCORRECT_OBJ_STATE (-35) /**< @brief Incorrect object state */ #define OS_ERR_INCORRECT_OBJ_TYPE (-36) /**< @brief Incorrect object type */ #define OS_ERR_STREAM_DISCONNECTED (-37) /**< @brief Stream disconnected */ +#define OS_ERR_OPERATION_NOT_SUPPORTED (-38) /**< @brief Requested operation is not support on the supplied object(s) */ /**@}*/ /* diff --git a/src/os/portable/os-impl-bsd-select.c b/src/os/portable/os-impl-bsd-select.c index b7282d811..abfbad33b 100644 --- a/src/os/portable/os-impl-bsd-select.c +++ b/src/os/portable/os-impl-bsd-select.c @@ -82,7 +82,7 @@ static int OS_FdSet_ConvertIn_Impl(fd_set *os_set, OS_FdSet *OSAL_set) { id = (offset * 8) + bit; osfd = OS_impl_filehandle_table[id].fd; - if (osfd >= 0) + if (osfd >= 0 && OS_impl_filehandle_table[id].selectable) { FD_SET(osfd, os_set); if (osfd > maxfd) @@ -247,6 +247,15 @@ int32 OS_SelectSingle_Impl(uint32 stream_id, uint32 *SelectFlags, int32 msecs) fd_set wr_set; fd_set rd_set; + /* + * If called on a stream_id which does not support this + * operation, return immediately and do not invoke the system call + */ + if (!OS_impl_filehandle_table[stream_id].selectable) + { + return OS_ERR_OPERATION_NOT_SUPPORTED; + } + if (*SelectFlags != 0) { FD_ZERO(&wr_set); @@ -304,6 +313,13 @@ int32 OS_SelectMultiple_Impl(OS_FdSet *ReadSet, OS_FdSet *WriteSet, int32 msecs) 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; @@ -324,7 +340,10 @@ int32 OS_SelectMultiple_Impl(OS_FdSet *ReadSet, OS_FdSet *WriteSet, int32 msecs) } } - return_code = OS_DoSelect(maxfd, &rd_set, &wr_set, msecs); + if (maxfd >= 0) + { + return_code = OS_DoSelect(maxfd, &rd_set, &wr_set, msecs); + } if (return_code == OS_SUCCESS) { diff --git a/src/os/shared/inc/os-shared-select.h b/src/os/shared/inc/os-shared-select.h index e7f83648f..0f0557429 100644 --- a/src/os/shared/inc/os-shared-select.h +++ b/src/os/shared/inc/os-shared-select.h @@ -42,6 +42,7 @@ Bits in "SelectFlags" will be unset according to activity Returns: OS_SUCCESS on success, or relevant error code + OS_ERR_OPERATION_NOT_SUPPORTED if the specified file handle does not support select ------------------------------------------------------------------*/ int32 OS_SelectSingle_Impl(uint32 stream_id, uint32 *SelectFlags, int32 msecs); @@ -66,6 +67,7 @@ int32 OS_SelectSingle_Impl(uint32 stream_id, uint32 *SelectFlags, int32 msecs); File descriptors in sets be modified according to activity Returns: OS_SUCCESS on success, or relevant error code + OS_ERR_OPERATION_NOT_SUPPORTED if the specified file handle(s) do not support select ------------------------------------------------------------------*/ int32 OS_SelectMultiple_Impl(OS_FdSet *ReadSet, OS_FdSet *WriteSet, int32 msecs); diff --git a/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c b/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c index 10d13d8de..5862eb09a 100644 --- a/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c +++ b/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c @@ -17,6 +17,7 @@ * */ #include "os-portable-coveragetest.h" +#include "ut-adaptor-portable-posix-io.h" #include "os-shared-select.h" #include @@ -32,7 +33,10 @@ void Test_OS_SelectSingle_Impl(void) struct OCS_timespec latertime; StreamID = 0; + UT_PortablePosixIOTest_Set_Selectable(0, false); SelectFlags = OS_STREAM_STATE_READABLE | OS_STREAM_STATE_WRITABLE; + OSAPI_TEST_FUNCTION_RC(OS_SelectSingle_Impl, (StreamID, &SelectFlags, 0), OS_ERR_OPERATION_NOT_SUPPORTED); + UT_PortablePosixIOTest_Set_Selectable(0, true); OSAPI_TEST_FUNCTION_RC(OS_SelectSingle_Impl, (StreamID, &SelectFlags, 0), OS_SUCCESS); SelectFlags = OS_STREAM_STATE_READABLE | OS_STREAM_STATE_WRITABLE; OSAPI_TEST_FUNCTION_RC(OS_SelectSingle_Impl, (StreamID, &SelectFlags, -1), OS_SUCCESS); diff --git a/src/unit-tests/oscore-test/ut_oscore_select_test.c b/src/unit-tests/oscore-test/ut_oscore_select_test.c index c69932147..13caa26ff 100644 --- a/src/unit-tests/oscore-test/ut_oscore_select_test.c +++ b/src/unit-tests/oscore-test/ut_oscore_select_test.c @@ -43,8 +43,8 @@ char *fsAddrPtr = NULL; static int32 setup_file(void) { - OS_mkfs(fsAddrPtr, "/ramdev3", " ", 512, 20); - OS_mount("/ramdev3", "/drive3"); + UT_SETUP(OS_mkfs(fsAddrPtr, "/ramdev3", "RAM3", 512, 20)); + UT_SETUP(OS_mount("/ramdev3", "/drive3")); return OS_creat("/drive3/select_test.txt", OS_READ_WRITE); } @@ -105,20 +105,25 @@ void UT_os_select_single_test(void) { uint32 StateFlags; int32 fd = setup_file(); + int32 rc; - if(OS_SelectSingle(fd, &StateFlags, 0) == OS_ERR_NOT_IMPLEMENTED) + UT_RETVAL(OS_SelectSingle(fd, NULL, 0), OS_INVALID_POINTER, "NULL flags pointer"); + + StateFlags = OS_STREAM_STATE_WRITABLE; + rc = OS_SelectSingle(fd, &StateFlags, 0); + if(rc == OS_ERR_NOT_IMPLEMENTED || rc == OS_ERR_OPERATION_NOT_SUPPORTED) { - UtAssertEx(false, UTASSERT_CASETYPE_NA, __FILE__, __LINE__, "OS_SelectSingle() not implemented"); + UtAssertEx(false, UTASSERT_CASETYPE_NA, __FILE__, __LINE__, "OS_SelectSingle() not supported"); goto UT_os_select_single_test_exit_tag; } - UtAssert_Simple(OS_SelectSingle(fd, NULL, 0) != OS_SUCCESS); - - StateFlags = OS_STREAM_STATE_WRITABLE; - UtAssert_Simple(OS_SelectSingle(fd, &StateFlags, 0) == OS_SUCCESS && StateFlags & OS_STREAM_STATE_WRITABLE); + UT_RETVAL(rc, OS_SUCCESS, "OS_SelectSingle(fd, OS_STREAM_STATE_WRITABLE, 0)"); + UtAssert_True((StateFlags & OS_STREAM_STATE_WRITABLE) != 0, "StateFlags (0x%x) & OS_STREAM_STATE_WRITABLE", (unsigned int)StateFlags); StateFlags = OS_STREAM_STATE_READABLE; - UtAssert_Simple(OS_SelectSingle(fd, &StateFlags, 1) == OS_SUCCESS); + rc = OS_SelectSingle(fd, &StateFlags, 1); + UT_RETVAL(rc, OS_SUCCESS, "OS_SelectSingle(fd, OS_STREAM_STATE_READABLE, 0)"); + UtAssert_True((StateFlags & OS_STREAM_STATE_READABLE) != 0, "StateFlags (0x%x) & OS_STREAM_STATE_READABLE", (unsigned int)StateFlags); UT_os_select_single_test_exit_tag: teardown_file(fd); @@ -135,25 +140,26 @@ void UT_os_select_multi_test(void) { OS_FdSet ReadSet, WriteSet; int32 fd = setup_file(); + int32 rc; - if(OS_SelectMultiple(&ReadSet, &WriteSet, 1) == OS_ERR_NOT_IMPLEMENTED) + OS_SelectFdZero(&WriteSet); + OS_SelectFdAdd(&WriteSet, fd); + rc = OS_SelectMultiple(NULL, &WriteSet, 1); + if(rc == OS_ERR_NOT_IMPLEMENTED || rc == OS_ERR_OPERATION_NOT_SUPPORTED) { - UtAssertEx(false, UTASSERT_CASETYPE_NA, __FILE__, __LINE__, "OS_SelectMultiple() not implemented"); + UtAssertEx(false, UTASSERT_CASETYPE_NA, __FILE__, __LINE__, "OS_SelectMultiple() not supported"); goto UT_select_multi_test_exit_tag; } - OS_SelectFdZero(&WriteSet); - OS_SelectFdAdd(&WriteSet, fd); - UtAssert_Simple(OS_SelectMultiple(NULL, &WriteSet, 1) == OS_SUCCESS); - OS_SelectFdZero(&ReadSet); - OS_SelectFdAdd(&ReadSet, fd); - UtAssert_Simple(OS_SelectMultiple(&ReadSet, NULL, 1) == OS_SUCCESS); + UT_RETVAL(rc, OS_SUCCESS, "OS_SelectMultiple(NULL, &WriteSet, 1)"); + UtAssert_True(OS_SelectFdIsSet(&WriteSet, fd), "OS_SelectFdIsSet(&WriteSet, fd)"); OS_SelectFdZero(&ReadSet); OS_SelectFdAdd(&ReadSet, fd); - OS_SelectFdZero(&WriteSet); - UtAssert_Simple(OS_SelectMultiple(&ReadSet, &WriteSet, 0) == OS_SUCCESS); + rc = OS_SelectMultiple(&ReadSet, NULL, 1); + UT_RETVAL(rc, OS_SUCCESS, "OS_SelectMultiple(&ReadSet, NULL, 1)"); + UtAssert_True(OS_SelectFdIsSet(&ReadSet, fd), "!OS_SelectFdIsSet(&ReadSet, fd)"); UT_select_multi_test_exit_tag: teardown_file(fd);