Skip to content

Commit

Permalink
Code improvements for the unresolved comments
Browse files Browse the repository at this point in the history 
This patch contains improvements mentioned in the unresolved PR
comments:
- function names were changed from socket_sendmsg/socket_recvmsg to
  socket_sendto_control/socket_recvfrom_control.
- default implementation of this functions was provided in the
  NetworkStack class.
- MsgHeaderIterator accesses elements on the aligned addresses.
  • Loading branch information
@mat-kalinowski
mat-kalinowski committed Sep 3, 2021
1 parent bdfd98e commit ec3f437
Show file tree
Hide file tree
Showing 23 changed files with 297 additions and 133 deletions.
12 changes: 6 additions & 6 deletions connectivity/cellular/include/cellular/framework/AT/AT_CellularStack.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,16 @@ class AT_CellularStack : public NetworkStack {
virtual void socket_attach(nsapi_socket_t handle, void (*callback)(void *), void *data);


nsapi_size_or_error_t socket_sendmsg(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override
nsapi_size_or_error_t socket_sendto_control(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override
{
return NSAPI_ERROR_UNSUPPORTED;
}

nsapi_size_or_error_t socket_recvmsg(nsapi_socket_t handle, SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override
nsapi_size_or_error_t socket_recvfrom_control(nsapi_socket_t handle, SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override
{
return NSAPI_ERROR_UNSUPPORTED;
}
Expand Down
14 changes: 7 additions & 7 deletions connectivity/lwipstack/include/lwipstack/LWIPStack.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,9 @@ class LWIP : public OnboardNetworkStack, private mbed::NonCopyable<LWIP> {
* @return Number of sent bytes on success, negative error
* code on failure
*/
nsapi_size_or_error_t socket_sendmsg(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;
nsapi_size_or_error_t socket_sendto_control(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;

/** Receive data over a TCP socket
*
Expand Down Expand Up @@ -519,7 +519,7 @@ class LWIP : public OnboardNetworkStack, private mbed::NonCopyable<LWIP> {
* This call is non-blocking. If recvfrom would block,
* NSAPI_ERROR_WOULD_BLOCK is returned immediately.
*
* It uses socket_recvmsg with zero ancillary data.
* It uses socket_recvfrom_control with zero ancillary data.
* @param handle Socket handle
* @param address Destination for the source address or NULL
* @param buffer Destination buffer for data received from the host
Expand Down Expand Up @@ -547,9 +547,9 @@ class LWIP : public OnboardNetworkStack, private mbed::NonCopyable<LWIP> {
* @return Number of received bytes on success, negative error
* code on failure
*/
nsapi_size_or_error_t socket_recvmsg(nsapi_socket_t handle, SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;
nsapi_size_or_error_t socket_recvfrom_control(nsapi_socket_t handle, SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;

/** Register a callback on state change of the socket
*
Expand Down
16 changes: 8 additions & 8 deletions connectivity/lwipstack/source/LWIPStack.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,18 +443,18 @@ nsapi_size_or_error_t LWIP::socket_recv(nsapi_socket_t handle, void *data, nsapi

nsapi_size_or_error_t LWIP::socket_sendto(nsapi_socket_t handle, const SocketAddress &address, const void *data, nsapi_size_t size)
{
return socket_sendmsg(handle, address, data, size, NULL, 0);
return socket_sendto_control(handle, address, data, size, NULL, 0);
}

nsapi_size_or_error_t LWIP::socket_recvfrom(nsapi_socket_t handle, SocketAddress *address, void *data, nsapi_size_t size)
{
return socket_recvmsg(handle, address, data, size, NULL, 0);
return socket_recvfrom_control(handle, address, data, size, NULL, 0);

}

nsapi_size_or_error_t LWIP::socket_recvmsg(nsapi_socket_t handle, SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size)
nsapi_size_or_error_t LWIP::socket_recvfrom_control(nsapi_socket_t handle, SocketAddress *address, void *data,
nsapi_size_t size, nsapi_msghdr_t *control,
nsapi_size_t control_size)
{
struct mbed_lwip_socket *s = (struct mbed_lwip_socket *)handle;
struct netbuf *buf;
Expand Down Expand Up @@ -491,9 +491,9 @@ nsapi_size_or_error_t LWIP::socket_recvmsg(nsapi_socket_t handle, SocketAddress
return recv;
}

nsapi_size_or_error_t LWIP::socket_sendmsg(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size)
nsapi_size_or_error_t LWIP::socket_sendto_control(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size, nsapi_msghdr_t *control,
nsapi_size_t control_size)
{
struct mbed_lwip_socket *s = (struct mbed_lwip_socket *)handle;
ip_addr_t ip_addr = {};
Expand Down
13 changes: 6 additions & 7 deletions connectivity/nanostack/include/nanostack-interface/Nanostack.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,17 +302,16 @@ class Nanostack : public OnboardNetworkStack, private mbed::NonCopyable<Nanostac
*/
nsapi_error_t getsockopt(void *handle, int level, int optname, void *optval, unsigned *optlen) override;

// FIXME: Implement
nsapi_size_or_error_t socket_sendmsg(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override
nsapi_size_or_error_t socket_sendto_control(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override
{
return NSAPI_ERROR_UNSUPPORTED;
}

nsapi_size_or_error_t socket_recvmsg(nsapi_socket_t handle, SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override
nsapi_size_or_error_t socket_recvfrom_control(nsapi_socket_t handle, SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override
{
return NSAPI_ERROR_UNSUPPORTED;
}
Expand Down
12 changes: 6 additions & 6 deletions connectivity/netsocket/include/netsocket/CellularNonIPSocket.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,13 @@ class CellularNonIPSocket : public Socket {
nsapi_size_or_error_t recvfrom(SocketAddress *address,
void *data, nsapi_size_t size) override;
/// NOT APPLICABLE
nsapi_size_or_error_t sendmsg(const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;
nsapi_size_or_error_t sendto_control(const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;
/// NOT APPLICABLE
nsapi_size_or_error_t recvmsg(SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;
nsapi_size_or_error_t recvfrom_control(SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;
/// NOT APPLICABLE
nsapi_error_t bind(const SocketAddress &address) override;

Expand Down
20 changes: 10 additions & 10 deletions connectivity/netsocket/include/netsocket/InternetDatagramSocket.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class InternetDatagramSocket : public InternetSocket {
* nonblocking or times out, NSAPI_ERROR_WOULD_BLOCK is returned
* immediately.
*
* It uses sendmsg with zero ancillary data
* It uses sendto_control with zero ancillary data
* @param address The SocketAddress of the remote host.
* @param data Buffer of data to send to the host.
* @param size Size of the buffer in bytes.
Expand All @@ -61,7 +61,7 @@ class InternetDatagramSocket : public InternetSocket {
* are accepted.
*
* @note recvfrom() is allowed write to address and data buffers even if error occurs.
* It uses recvmsg with zero ancillary data
* It uses recvfrom_control with zero ancillary data
* @param address Destination for the source address or NULL.
* @param data Destination buffer for RAW data to be received from the host.
* @param size Size of the buffer in bytes.
Expand All @@ -81,7 +81,7 @@ class InternetDatagramSocket : public InternetSocket {
* nonblocking or times out, NSAPI_ERROR_WOULD_BLOCK is returned
* immediately.
*
* It uses sendmsg with zero ancillary data
* It uses sendto_control with zero ancillary data
* @param address The SocketAddress of the remote host.
* @param data Buffer of data to send to the host.
* @param size Size of the buffer in bytes.
Expand All @@ -93,9 +93,9 @@ class InternetDatagramSocket : public InternetSocket {
* @retval int Other negative error codes for stack-related failures.
* See \ref NetworkStack::socket_send.
*/
nsapi_size_or_error_t sendmsg(const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;
nsapi_size_or_error_t sendto_control(const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;


/** Receive a datagram with ancillary data and store the source address in address if it's not NULL.
Expand All @@ -109,7 +109,7 @@ class InternetDatagramSocket : public InternetSocket {
* @note If socket is connected, only packets coming from connected peer address
* are accepted.
*
* @note recvmsg() is allowed write to address and data buffers even if error occurs.
* @note recvfrom_control() is allowed write to address and data buffers even if error occurs.
*
* @param address Destination for the source address or NULL.
* @param data Destination buffer for RAW data to be received from the host.
Expand All @@ -123,9 +123,9 @@ class InternetDatagramSocket : public InternetSocket {
* @retval int Other negative error codes for stack-related failures.
* See \ref NetworkStack::socket_recv.
*/
nsapi_size_or_error_t recvmsg(SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;
nsapi_size_or_error_t recvfrom_control(SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) override;

/** Set the remote address for next send() call and filtering
* of incoming packets. To reset the address, zero initialized
Expand Down
31 changes: 28 additions & 3 deletions connectivity/netsocket/include/netsocket/MsgHeader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,24 @@

#include "netsocket/nsapi_types.h"

/**
* Allows iteration through the list of message headers received in the control parameter of the
* socket_sendto_control / socket_recvfrom_control methods.
*/

struct MsgHeaderIterator {
// Constructor takes pointer to the first header element and size of the whole list.
MsgHeaderIterator(nsapi_msghdr_t *hdr, nsapi_size_t size) :
start(hdr),
current(nullptr),
size(size)
{}

// Checks if the next address of the iterator is a valid list member.
bool has_next()
{
if (current == nullptr) {
if (start != nullptr) {
if (start != nullptr && start->len <= size && start->len >= sizeof(*start)) {
return true;
} else {
return false;
Expand All @@ -41,13 +48,15 @@ struct MsgHeaderIterator {
return false;
}

if ((reinterpret_cast<uint8_t *>(current) + current->len) >= (reinterpret_cast<uint8_t *>(start) + size)) {
if (get_next_aligned_addr() >= (reinterpret_cast<uint8_t *>(start) + size)) {
return false;
}

return true;
}

// Returns pointer to the next member of the list.
// If next member doesn't exist nullptr is returned.
nsapi_msghdr_t *next()
{
if (!has_next()) {
Expand All @@ -57,13 +66,29 @@ struct MsgHeaderIterator {
if (current == nullptr) {
current = start;
} else {
current = reinterpret_cast<nsapi_msghdr_t *>(reinterpret_cast<uint8_t *>(current) + current->len);
current = reinterpret_cast<nsapi_msghdr *>(get_next_aligned_addr());
}

return current;
}

private:
// Get address of the next member aligned to the size of msghdr_t.
void *get_next_aligned_addr()
{
size_t remaining_size = size - (reinterpret_cast<uintptr_t>(current) - reinterpret_cast<uintptr_t>(start));
void *next = reinterpret_cast<void *>(reinterpret_cast<uint8_t *>(current) + current->len);

next = std::align(
alignof(nsapi_msghdr_t),
sizeof(nsapi_msghdr_t),
next,
remaining_size
);

return next;
}

nsapi_msghdr_t *start;
nsapi_msghdr_t *current;
nsapi_size_t size;
Expand Down
26 changes: 20 additions & 6 deletions connectivity/netsocket/include/netsocket/NetworkStack.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,9 +412,16 @@ class NetworkStack : public DNS {
* @return Number of sent bytes on success, negative error
* code on failure
*/
virtual nsapi_size_or_error_t socket_sendmsg(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) = 0;
virtual nsapi_size_or_error_t socket_sendto_control(nsapi_socket_t handle, const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size)
{
if (control != NULL) {
return NSAPI_ERROR_UNSUPPORTED;
}

return socket_sendto(handle, address, data, size);
}

/** Receive a packet with ancillary data over a UDP socket
*
Expand All @@ -436,9 +443,16 @@ class NetworkStack : public DNS {
* @return Number of received bytes on success, negative error
* code on failure
*/
virtual nsapi_size_or_error_t socket_recvmsg(nsapi_socket_t handle, SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) = 0;
virtual nsapi_size_or_error_t socket_recvfrom_control(nsapi_socket_t handle, SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size)
{
if (control != NULL) {
return NSAPI_ERROR_UNSUPPORTED;
}

return socket_recvfrom(handle, address, data, size);
}

/** Register a callback on state change of the socket
*
Expand Down
16 changes: 8 additions & 8 deletions connectivity/netsocket/include/netsocket/Socket.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class Socket {

/** Send a message on a socket.
*
* The sendmsg() function sends a message through a connection-mode or connectionless-mode socket.
* The sendto_control() function sends a message through a connection-mode or connectionless-mode socket.
* If the socket is a connectionless-mode socket, the message is sent to the address specified.
* If the socket is a connected-mode socket, address is ignored.
*
Expand All @@ -175,9 +175,9 @@ class Socket {
* @return Number of sent bytes on success, negative subclass-dependent error
* code on failure
*/
virtual nsapi_size_or_error_t sendmsg(const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) = 0;
virtual nsapi_size_or_error_t sendto_control(const SocketAddress &address,
const void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) = 0;


/** Receive a data from a socket
Expand All @@ -190,7 +190,7 @@ class Socket {
*
* Additional information related to the message can be retrieved with the control data.
*
* @note recvmsg() is allowed write to address and data buffers even if error occurs.
* @note recvfrom_control() is allowed write to address and data buffers even if error occurs.
*
* By default, recvfrom blocks until a datagram is received. If socket is set to
* non-blocking or times out with no data, NSAPI_ERROR_WOULD_BLOCK
Expand All @@ -202,9 +202,9 @@ class Socket {
* @return Number of received bytes on success, negative subclass-dependent
* error code on failure
*/
virtual nsapi_size_or_error_t recvmsg(SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) = 0;
virtual nsapi_size_or_error_t recvfrom_control(SocketAddress *address,
void *data, nsapi_size_t size,
nsapi_msghdr_t *control, nsapi_size_t control_size) = 0;

/** Bind a specific address to a socket.
*
Expand Down
Loading

0 comments on commit ec3f437

Please sign in to comment.