Skip to content

Commit

Permalink
Fix #453, Select API and unit test updates
Browse files Browse the repository at this point in the history
Confirm that the "selectable" flag is set before
calling the underlying select() API.

Also update unit tests to match.
  • Loading branch information
jphickey committed May 15, 2020
1 parent e237ab3 commit aade493
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 21 deletions.
1 change: 1 addition & 0 deletions src/os/inc/osapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) */
/**@}*/

/*
Expand Down
23 changes: 21 additions & 2 deletions src/os/portable/os-impl-bsd-select.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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)
{
Expand Down
2 changes: 2 additions & 0 deletions src/os/shared/inc/os-shared-select.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down
4 changes: 4 additions & 0 deletions src/unit-test-coverage/portable/src/coveragetest-bsd-select.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*
*/
#include "os-portable-coveragetest.h"
#include "ut-adaptor-portable-posix-io.h"
#include "os-shared-select.h"

#include <OCS_sys_select.h>
Expand All @@ -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);
Expand Down
44 changes: 25 additions & 19 deletions src/unit-tests/oscore-test/ut_oscore_select_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit aade493

Please sign in to comment.