Skip to content

Commit

Permalink
Fix #1398, separate bind and listen calls for streams
Browse files Browse the repository at this point in the history
Add OS_SocketBindAddress() and OS_SocketListen() to allow users to bind
an address without also calling listen().  The use case is for client
side connections where the source port needs to be controlled.
  • Loading branch information
jphickey committed Jul 27, 2023
1 parent 184ba9b commit 1747e64
Show file tree
Hide file tree
Showing 32 changed files with 448 additions and 134 deletions.
3 changes: 2 additions & 1 deletion src/os/inc/osapi-select.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ typedef enum
OS_STREAM_STATE_BOUND = 0x01, /**< @brief whether the stream is bound */
OS_STREAM_STATE_CONNECTED = 0x02, /**< @brief whether the stream is connected */
OS_STREAM_STATE_READABLE = 0x04, /**< @brief whether the stream is readable */
OS_STREAM_STATE_WRITABLE = 0x08 /**< @brief whether the stream is writable */
OS_STREAM_STATE_WRITABLE = 0x08, /**< @brief whether the stream is writable */
OS_STREAM_STATE_LISTENING = 0x10 /**< @brief whether the stream is listening */
} OS_StreamState_t;

/** @defgroup OSAPISelect OSAL Select APIs
Expand Down
50 changes: 45 additions & 5 deletions src/os/inc/osapi-sockets.h
Original file line number Diff line number Diff line change
Expand Up @@ -274,15 +274,14 @@ int32 OS_SocketOpen(osal_id_t *sock_id, OS_SocketDomain_t Domain, OS_SocketType_

/*-------------------------------------------------------------------------------------*/
/**
* @brief Binds a socket to a given local address.
* @brief Binds a socket to a given local address and enter listening (server) mode.
*
* The specified socket will be bound to the local address and port, if available.
* This is a convenience/compatibility routine to perform both OS_SocketBindAddress() and
* OS_SocketListen() operations in a single call, intended to simplify the setup for a server
* role.
*
* If the socket is connectionless, then it only binds to the local address.
*
* If the socket is connection-oriented (stream), then this will also put the
* socket into a listening state for incoming connections at the local address.
*
* @param[in] sock_id The socket ID
* @param[in] Addr The local address to bind to @nonnull
*
Expand All @@ -295,6 +294,47 @@ int32 OS_SocketOpen(osal_id_t *sock_id, OS_SocketDomain_t Domain, OS_SocketType_
*/
int32 OS_SocketBind(osal_id_t sock_id, const OS_SockAddr_t *Addr);

/*-------------------------------------------------------------------------------------*/
/**
* @brief Places the specified socket into a listening state.
*
* This function only applies to connection-oriented (stream) sockets that are intended
* to be used in a server-side role. This places the socket into a state where it can
* accept incoming connections from clients.
*
* @param[in] sock_id The socket ID
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
* @retval #OS_ERR_INVALID_ID if the sock_id parameter is not valid
* @retval #OS_ERR_INCORRECT_OBJ_STATE if the socket is already listening
* @retval #OS_ERR_INCORRECT_OBJ_TYPE if the handle is not a stream socket
*/
int32 OS_SocketListen(osal_id_t sock_id);

/*-------------------------------------------------------------------------------------*/
/**
* @brief Binds a socket to a given local address.
*
* The specified socket will be bound to the local address and port, if available.
* This controls the source address reflected in network traffic transmitted via this
* socket.
*
* After binding to the address, a stream socket may be followed by a call to either
* OS_SocketListen() for a server role or to OS_SocketConnect() for a client role.
*
* @param[in] sock_id The socket ID
* @param[in] Addr The local address to bind to @nonnull
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
* @retval #OS_ERR_INVALID_ID if the sock_id parameter is not valid
* @retval #OS_INVALID_POINTER if argument is NULL
* @retval #OS_ERR_INCORRECT_OBJ_STATE if the socket is already bound
* @retval #OS_ERR_INCORRECT_OBJ_TYPE if the handle is not a socket
*/
int32 OS_SocketBindAddress(osal_id_t sock_id, const OS_SockAddr_t *Addr);

/*-------------------------------------------------------------------------------------*/
/**
* @brief Connects a socket to a given remote address.
Expand Down
35 changes: 23 additions & 12 deletions src/os/portable/os-impl-bsd-sockets.c
Original file line number Diff line number Diff line change
Expand Up @@ -214,16 +214,14 @@ int32 OS_SocketOpen_Impl(const OS_object_token_t *token)
* See prototype for argument/return detail
*
*-----------------------------------------------------------------*/
int32 OS_SocketBind_Impl(const OS_object_token_t *token, const OS_SockAddr_t *Addr)
int32 OS_SocketBindAddress_Impl(const OS_object_token_t *token, const OS_SockAddr_t *Addr)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
int os_result;
socklen_t addrlen;
const struct sockaddr * sa;
OS_impl_file_internal_record_t *impl;
OS_stream_internal_record_t * stream;

impl = OS_OBJECT_TABLE_GET(OS_impl_filehandle_table, *token);
stream = OS_OBJECT_TABLE_GET(OS_stream_table, *token);
impl = OS_OBJECT_TABLE_GET(OS_impl_filehandle_table, *token);

sa = (const struct sockaddr *)&Addr->AddrData;

Expand Down Expand Up @@ -254,16 +252,29 @@ int32 OS_SocketBind_Impl(const OS_object_token_t *token, const OS_SockAddr_t *Ad
return OS_ERROR;
}

/* Start listening on the socket (implied for stream sockets) */
if (stream->socket_type == OS_SocketType_STREAM)
return OS_SUCCESS;
}

/*----------------------------------------------------------------
*
* Purpose: Implemented per internal OSAL API
* See prototype for argument/return detail
*
*-----------------------------------------------------------------*/
int32 OS_SocketListen_Impl(const OS_object_token_t *token)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
int os_result;
OS_impl_file_internal_record_t *impl;

impl = OS_OBJECT_TABLE_GET(OS_impl_filehandle_table, *token);

os_result = listen(impl->fd, 10);
if (os_result < 0)
{
os_result = listen(impl->fd, 10);
if (os_result < 0)
{
OS_DEBUG("listen: %s\n", strerror(errno));
return OS_ERROR;
}
OS_DEBUG("listen: %s\n", strerror(errno));
return OS_ERROR;
}

return OS_SUCCESS;
}

Expand Down
12 changes: 11 additions & 1 deletion src/os/portable/os-impl-no-sockets.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,17 @@ int32 OS_SocketOpen_Impl(const OS_object_token_t *token)
*
* See prototype for argument/return detail
*-----------------------------------------------------------------*/
int32 OS_SocketBind_Impl(const OS_object_token_t *token, const OS_SockAddr_t *Addr)
int32 OS_SocketBindAddress_Impl(const OS_object_token_t *token, const OS_SockAddr_t *Addr)
{
return OS_ERR_NOT_IMPLEMENTED;
}

/*----------------------------------------------------------------
* Implementation for no network configuration
*
* See prototype for argument/return detail
*-----------------------------------------------------------------*/
int32 OS_SocketListen_Impl(const OS_object_token_t *token)
{
return OS_ERR_NOT_IMPLEMENTED;
}
Expand Down
10 changes: 9 additions & 1 deletion src/os/shared/inc/os-shared-sockets.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,15 @@ int32 OS_SocketOpen_Impl(const OS_object_token_t *token);
Returns: OS_SUCCESS on success, or relevant error code
------------------------------------------------------------------*/
int32 OS_SocketBind_Impl(const OS_object_token_t *token, const OS_SockAddr_t *Addr);
int32 OS_SocketBindAddress_Impl(const OS_object_token_t *token, const OS_SockAddr_t *Addr);

/*----------------------------------------------------------------
Purpose: Binds the indicated socket table entry to the passed-in address
Returns: OS_SUCCESS on success, or relevant error code
------------------------------------------------------------------*/
int32 OS_SocketListen_Impl(const OS_object_token_t *token);

/*----------------------------------------------------------------
Expand Down
81 changes: 80 additions & 1 deletion src/os/shared/src/osapi-sockets.c
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,87 @@ int32 OS_SocketOpen(osal_id_t *sock_id, OS_SocketDomain_t Domain, OS_SocketType_
* Purpose: Implemented per public OSAL API
* See description in API and header file for detail
*
* This is now just a convenience/shorthand routine handling both bind
* and listen, preserved for backward compatibility.
*
*-----------------------------------------------------------------*/
int32 OS_SocketBind(osal_id_t sock_id, const OS_SockAddr_t *Addr)
{
int32 return_code;

return_code = OS_SocketBindAddress(sock_id, Addr);
if (return_code == OS_SUCCESS)
{
return_code = OS_SocketListen(sock_id);
if (return_code == OS_ERR_INCORRECT_OBJ_TYPE)
{
/* This one is OK, it happens if the socket is a datagram/connectionless
* type that does not need to listen(). For backward compatibility, report
* success to the caller.
*/
return_code = OS_SUCCESS;
}
}

return return_code;
}

/*----------------------------------------------------------------
*
* Purpose: Implemented per public OSAL API
* See description in API and header file for detail
*
*-----------------------------------------------------------------*/
int32 OS_SocketListen(osal_id_t sock_id)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
OS_stream_internal_record_t *stream;
OS_object_token_t token;
int32 return_code;

return_code = OS_ObjectIdGetById(OS_LOCK_MODE_EXCLUSIVE, LOCAL_OBJID_TYPE, sock_id, &token);
if (return_code == OS_SUCCESS)
{
stream = OS_OBJECT_TABLE_GET(OS_stream_table, token);

/* This call is only applicable to stream sockets */
if (stream->socket_domain == OS_SocketDomain_INVALID || stream->socket_type != OS_SocketType_STREAM)
{
/* Not a stream socket */
return_code = OS_ERR_INCORRECT_OBJ_TYPE;
}
else if ((stream->stream_state & OS_STREAM_STATE_BOUND) == 0)
{
/* Socket must be bound to an address already */
return_code = OS_ERR_INCORRECT_OBJ_STATE;
}
else if ((stream->stream_state & (OS_STREAM_STATE_LISTENING | OS_STREAM_STATE_CONNECTED)) != 0)
{
/* Socket must be neither listening nor connected */
return_code = OS_ERR_INCORRECT_OBJ_STATE;
}
else
{
return_code = OS_SocketListen_Impl(&token);

if (return_code == OS_SUCCESS)
{
stream->stream_state |= OS_STREAM_STATE_LISTENING;
}
}

OS_ObjectIdRelease(&token);
}

return return_code;
}

/*----------------------------------------------------------------
*
* Purpose: Implemented per public OSAL API
* See description in API and header file for detail
*
*-----------------------------------------------------------------*/
int32 OS_SocketBindAddress(osal_id_t sock_id, const OS_SockAddr_t *Addr)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
OS_common_record_t * record;
OS_stream_internal_record_t *stream;
Expand Down Expand Up @@ -180,7 +259,7 @@ int32 OS_SocketBind(osal_id_t sock_id, const OS_SockAddr_t *Addr)
}
else
{
return_code = OS_SocketBind_Impl(&token, Addr);
return_code = OS_SocketBindAddress_Impl(&token, Addr);

if (return_code == OS_SUCCESS)
{
Expand Down
40 changes: 26 additions & 14 deletions src/unit-test-coverage/portable/src/coveragetest-bsd-sockets.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ void Test_OS_SetSocketDefaultFlags_Impl(void)
UtAssert_True(UT_PortablePosixIOTest_Get_Selectable(token.obj_idx), "Socket is selectable");
}

void Test_OS_SocketBind_Impl(void)
void Test_OS_SocketBindAddress_Impl(void)
{
OS_object_token_t token = {0};
OS_SockAddr_t addr = {0};
Expand All @@ -139,28 +139,39 @@ void Test_OS_SocketBind_Impl(void)

/* Default family case */
sa->sa_family = -1;
OSAPI_TEST_FUNCTION_RC(OS_SocketBind_Impl, (&token, &addr), OS_ERR_BAD_ADDRESS);
OSAPI_TEST_FUNCTION_RC(OS_SocketBindAddress_Impl, (&token, &addr), OS_ERR_BAD_ADDRESS);

/* Note - not attempting to hit addrlen > OS_SOCKADDR_MAX_LEN at this point (NOT MC/DC)
* would require compiling with a small OS_SOCKADDR_MAX_LEN or bigger structure */

/* Fail bind */
sa->sa_family = OCS_AF_INET;
UT_SetDeferredRetcode(UT_KEY(OCS_bind), 1, -1);
OSAPI_TEST_FUNCTION_RC(OS_SocketBind_Impl, (&token, &addr), OS_ERROR);
OSAPI_TEST_FUNCTION_RC(OS_SocketBindAddress_Impl, (&token, &addr), OS_ERROR);

/* Fail listen */
sa->sa_family = OCS_AF_INET6;
OS_stream_table[0].socket_type = OS_SocketType_STREAM;
UT_SetDeferredRetcode(UT_KEY(OCS_listen), 1, -1);
OSAPI_TEST_FUNCTION_RC(OS_SocketBind_Impl, (&token, &addr), OS_ERROR);
/* Success with INET address */
sa->sa_family = OCS_AF_INET;
OSAPI_TEST_FUNCTION_RC(OS_SocketBindAddress_Impl, (&token, &addr), OS_SUCCESS);

/* Success with INET6 address */
sa->sa_family = OCS_AF_INET6;
OSAPI_TEST_FUNCTION_RC(OS_SocketBindAddress_Impl, (&token, &addr), OS_SUCCESS);
}

void Test_OS_SocketListen_Impl(void)
{
OS_object_token_t token = {0};

/* Set up token for index 0 */
token.obj_idx = UT_INDEX_0;

/* Success with OS_SocketType_STREAM */
OSAPI_TEST_FUNCTION_RC(OS_SocketBind_Impl, (&token, &addr), OS_SUCCESS);
/* Nominal Success */
OS_stream_table[0].socket_type = OS_SocketType_STREAM;
OSAPI_TEST_FUNCTION_RC(OS_SocketListen_Impl, (&token), OS_SUCCESS);

/* Success with ~OS_SocketType_STREAM */
OS_stream_table[0].socket_type = ~OS_SocketType_STREAM;
OSAPI_TEST_FUNCTION_RC(OS_SocketBind_Impl, (&token, &addr), OS_SUCCESS);
/* Fail listen */
UT_SetDeferredRetcode(UT_KEY(OCS_listen), 1, -1);
OSAPI_TEST_FUNCTION_RC(OS_SocketListen_Impl, (&token), OS_ERROR);
}

void Test_OS_SocketConnect_Impl(void)
Expand Down Expand Up @@ -482,7 +493,8 @@ void UtTest_Setup(void)
{
ADD_TEST(OS_SocketOpen_Impl);
ADD_TEST(OS_SetSocketDefaultFlags_Impl);
ADD_TEST(OS_SocketBind_Impl);
ADD_TEST(OS_SocketBindAddress_Impl);
ADD_TEST(OS_SocketListen_Impl);
ADD_TEST(OS_SocketConnect_Impl);
ADD_TEST(OS_SocketShutdown_Impl);
ADD_TEST(OS_SocketAccept_Impl);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
void Test_No_Sockets(void)
{
OSAPI_TEST_FUNCTION_RC(OS_SocketOpen_Impl, (NULL), OS_ERR_NOT_IMPLEMENTED);
OSAPI_TEST_FUNCTION_RC(OS_SocketBind_Impl, (NULL, NULL), OS_ERR_NOT_IMPLEMENTED);
OSAPI_TEST_FUNCTION_RC(OS_SocketBindAddress_Impl, (NULL, NULL), OS_ERR_NOT_IMPLEMENTED);
OSAPI_TEST_FUNCTION_RC(OS_SocketListen_Impl, (NULL), OS_ERR_NOT_IMPLEMENTED);
OSAPI_TEST_FUNCTION_RC(OS_SocketConnect_Impl, (NULL, NULL, 0), OS_ERR_NOT_IMPLEMENTED);
OSAPI_TEST_FUNCTION_RC(OS_SocketAccept_Impl, (NULL, NULL, NULL, 0), OS_ERR_NOT_IMPLEMENTED);
OSAPI_TEST_FUNCTION_RC(OS_SocketShutdown_Impl, (NULL, 0), OS_ERR_NOT_IMPLEMENTED);
Expand Down
Loading

0 comments on commit 1747e64

Please sign in to comment.