Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Open Port Request on Sending Side of IpSocket #2683

Open
wants to merge 10 commits into
base: devel
Choose a base branch
from
1 change: 1 addition & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@
crcstat
CREATEDIRECTORY
Crosscompiling
crsmith
crt
CRTSCTS
cryptsoft
Expand Down Expand Up @@ -1013,9 +1014,9 @@
subseconds
subtargets
subtopology
subtopologies

Check warning on line 1017 in .github/actions/spelling/expect.txt

View workflow job for this annotation

GitHub Actions / Spell checking

`subtopologies` is ignored by check spelling because another more general variant is also in expect. (ignored-expect-variant)
Subtopology

Check warning on line 1018 in .github/actions/spelling/expect.txt

View workflow job for this annotation

GitHub Actions / Spell checking

`Subtopology` is ignored by check spelling because another more general variant is also in expect. (ignored-expect-variant)
Subtopologies

Check warning on line 1019 in .github/actions/spelling/expect.txt

View workflow job for this annotation

GitHub Actions / Spell checking

`Subtopologies` is ignored by check spelling because another more general variant is also in expect. (ignored-expect-variant)
suppr
suseconds
SVCLOGFILE
Expand Down Expand Up @@ -1212,3 +1213,3 @@
xsltproc
xxxx
yacgen
Expand Down
2 changes: 1 addition & 1 deletion Drv/Ip/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ set(SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/TcpClientSocket.cpp"
"${CMAKE_CURRENT_LIST_DIR}/TcpServerSocket.cpp"
"${CMAKE_CURRENT_LIST_DIR}/UdpSocket.cpp"
"${CMAKE_CURRENT_LIST_DIR}/SocketReadTask.cpp"
"${CMAKE_CURRENT_LIST_DIR}/SocketComponentHelper.cpp"
)

set(MOD_DEPS
Expand Down
83 changes: 15 additions & 68 deletions Drv/Ip/IpSocket.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// ======================================================================
// \title IpSocket.cpp
// \author mstarch
// \author mstarch, crsmith
// \brief cpp file for IpSocket core implementation classes
//
// \copyright
Expand Down Expand Up @@ -46,20 +46,18 @@

namespace Drv {

IpSocket::IpSocket() : m_fd(-1), m_timeoutSeconds(0), m_timeoutMicroseconds(0), m_port(0), m_open(false), m_started(false) {
IpSocket::IpSocket() : m_timeoutSeconds(0), m_timeoutMicroseconds(0), m_port(0) {
::memset(m_hostname, 0, sizeof(m_hostname));
}

SocketIpStatus IpSocket::configure(const char* const hostname, const U16 port, const U32 timeout_seconds, const U32 timeout_microseconds) {
FW_ASSERT(timeout_microseconds < 1000000, static_cast<FwAssertArgType>(timeout_microseconds));
FW_ASSERT(this->isValidPort(port));
FW_ASSERT(hostname != nullptr);
this->m_lock.lock();
this->m_timeoutSeconds = timeout_seconds;
this->m_timeoutMicroseconds = timeout_microseconds;
this->m_port = port;
(void) Fw::StringUtils::string_copy(this->m_hostname, hostname, static_cast<FwSizeType>(SOCKET_MAX_HOSTNAME_SIZE));
this->m_lock.unlock();
return SOCK_SUCCESS;
}

Expand Down Expand Up @@ -105,88 +103,45 @@
return SOCK_SUCCESS;
}

bool IpSocket::isStarted() {
bool is_started = false;
this->m_lock.lock();
is_started = this->m_started;
this->m_lock.unLock();
return is_started;
void IpSocket::close(NATIVE_INT_TYPE fd) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

fd uses the basic integral type int rather than a typedef with size and signedness.
(void)::shutdown(fd, SHUT_RDWR);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter fd has not been checked.
(void)::close(fd);
}

bool IpSocket::isOpened() {
bool is_open = false;
this->m_lock.lock();
is_open = this->m_open;
this->m_lock.unLock();
return is_open;
}

void IpSocket::close() {
this->m_lock.lock();
if (this->m_fd != -1) {
(void)::shutdown(this->m_fd, SHUT_RDWR);
(void)::close(this->m_fd);
this->m_fd = -1;
}
this->m_open = false;
this->m_lock.unLock();
}

void IpSocket::shutdown() {
this->close();
this->m_lock.lock();
this->m_started = false;
this->m_lock.unLock();
void IpSocket::shutdown(NATIVE_INT_TYPE fd) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

fd uses the basic integral type int rather than a typedef with size and signedness.
this->close(fd);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter fd has not been checked.
}

SocketIpStatus IpSocket::startup() {
this->m_lock.lock();
this->m_started = true;
this->m_lock.unLock();
// no op for non-server components
return SOCK_SUCCESS;
}

SocketIpStatus IpSocket::open() {
NATIVE_INT_TYPE fd = -1;
SocketIpStatus IpSocket::open(NATIVE_INT_TYPE& fd) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

fd uses the basic integral type int rather than a typedef with size and signedness.
SocketIpStatus status = SOCK_SUCCESS;
this->m_lock.lock();
FW_ASSERT(m_fd == -1 and not m_open); // Ensure we are not opening an opened socket
this->m_lock.unlock();
// Open a TCP socket for incoming commands, and outgoing data if not using UDP
status = this->openProtocol(fd);
if (status != SOCK_SUCCESS) {
FW_ASSERT(m_fd == -1); // Ensure we properly kept closed on error
// TODO: move this to SocketComponentHelper?
FW_ASSERT(fd == -1); // Ensure we properly kept closed on error
return status;
}
// Lock to update values and "officially open"
this->m_lock.lock();
this->m_fd = fd;
this->m_open = true;
this->m_lock.unLock();
return status;
}

SocketIpStatus IpSocket::send(const U8* const data, const U32 size) {
SocketIpStatus IpSocket::send(NATIVE_INT_TYPE fd, const U8* const data, const U32 size) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

fd uses the basic integral type int rather than a typedef with size and signedness.

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
U32 total = 0;
I32 sent = 0;
this->m_lock.lock();
NATIVE_INT_TYPE fd = this->m_fd;
this->m_lock.unlock();
// Prevent transmission before connection, or after a disconnect
if (fd == -1) {
return SOCK_DISCONNECTED;
}
// Attempt to send out data and retry as necessary
for (U32 i = 0; (i < SOCKET_MAX_ITERATIONS) && (total < size); i++) {
// Send using my specific protocol
sent = this->sendProtocol(data + total, size - total);
sent = this->sendProtocol(fd, data + total, size - total);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter fd has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter data has not been checked.
// Error is EINTR or timeout just try again
if (((sent == -1) && (errno == EINTR)) || (sent == 0)) {
continue;
}
// Error bad file descriptor is a close along with reset
else if ((sent == -1) && ((errno == EBADF) || (errno == ECONNRESET))) {
this->close();
return SOCK_DISCONNECTED;
}
// Error returned, and it wasn't an interrupt nor a disconnect
Expand All @@ -205,27 +160,19 @@
return SOCK_SUCCESS;
}

SocketIpStatus IpSocket::recv(U8* data, U32& req_read) {
SocketIpStatus IpSocket::recv(NATIVE_INT_TYPE fd, U8* data, U32& req_read) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

fd uses the basic integral type int rather than a typedef with size and signedness.

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
I32 size = 0;
// Check for previously disconnected socket
this->m_lock.lock();
NATIVE_INT_TYPE fd = this->m_fd;
this->m_lock.unlock();
if (fd == -1) {
return SOCK_DISCONNECTED;
}

// Try to read until we fail to receive data
for (U32 i = 0; (i < SOCKET_MAX_ITERATIONS) && (size <= 0); i++) {
// Attempt to recv out data
size = this->recvProtocol(data, req_read);
size = this->recvProtocol(fd, data, req_read);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter fd has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter data has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter req_read has not been checked.
// Error is EINTR, just try again
if (size == -1 && ((errno == EINTR) || errno == EAGAIN)) {
continue;
}
// Zero bytes read reset or bad ef means we've disconnected
else if (size == 0 || ((size == -1) && ((errno == ECONNRESET) || (errno == EBADF)))) {
this->close();
req_read = static_cast<U32>(size);
return SOCK_DISCONNECTED;
}
Expand Down
45 changes: 16 additions & 29 deletions Drv/Ip/IpSocket.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,24 +68,6 @@ class IpSocket {
*/
SocketIpStatus configure(const char* hostname, const U16 port, const U32 send_timeout_seconds,
const U32 send_timeout_microseconds);
/**
* \brief Returns true when the socket is started
*
* Returns true when the socket is started up sufficiently to be actively listening to clients. Returns false
* otherwise. This means `startup()` was called and returned success.
*/
bool isStarted();

/**
* \brief check if IP socket has previously been opened
*
* Check if this IpSocket has previously been opened. In the case of Udp this will check for outgoing transmissions
* and (if configured) incoming transmissions as well. This does not guarantee errors will not occur when using this
* socket as the remote component may have disconnected.
*
* \return true if socket is open, false otherwise
*/
bool isOpened();

/**
* \brief startup the socket, a no-op on unless this is server
Expand All @@ -112,9 +94,10 @@ class IpSocket {
*
* Note: delegates to openProtocol for protocol specific implementation
*
* \param fd: file descriptor to open
* \return status of open
*/
SocketIpStatus open();
SocketIpStatus open(NATIVE_INT_TYPE& fd);
/**
* \brief send data out the IP socket from the given buffer
*
Expand All @@ -126,11 +109,12 @@ class IpSocket {
*
* Note: delegates to `sendProtocol` to send the data
*
* \param fd: file descriptor to send to
* \param data: pointer to data to send
* \param size: size of data to send
* \return status of the send, SOCK_DISCONNECTED to reopen, SOCK_SUCCESS on success, something else on error
*/
SocketIpStatus send(const U8* const data, const U32 size);
SocketIpStatus send(NATIVE_INT_TYPE fd, const U8* const data, const U32 size);
/**
* \brief receive data from the IP socket from the given buffer
*
Expand All @@ -142,26 +126,31 @@ class IpSocket {
*
* Note: delegates to `recvProtocol` to send the data
*
* \param fd: file descriptor to recv from
* \param data: pointer to data to fill with received data
* \param size: maximum size of data buffer to fill
* \return status of the send, SOCK_DISCONNECTED to reopen, SOCK_SUCCESS on success, something else on error
*/
SocketIpStatus recv(U8* const data, U32& size);
SocketIpStatus recv(NATIVE_INT_TYPE fd, U8* const data, U32& size);
/**
* \brief closes the socket
*
* Closes the socket opened by the open call. In this case of the TcpServer, this does NOT close server's listening
* port (call `shutdown`) but will close the active client connection.
*
* \param fd: file descriptor to close
*/
void close();
void close(NATIVE_INT_TYPE fd);

/**
* \brief shutdown the socket
*
* Closes the socket opened by the open call. In this case of the TcpServer, this does close server's listening
* port. This will shutdown all clients.
*
* \param fd: file descriptor to shutdown
*/
virtual void shutdown();
virtual void shutdown(NATIVE_INT_TYPE fd);

PROTECTED:
/**
Expand Down Expand Up @@ -198,27 +187,25 @@ class IpSocket {
virtual SocketIpStatus openProtocol(NATIVE_INT_TYPE& fd) = 0;
/**
* \brief Protocol specific implementation of send. Called directly with retry from send.
* \param fd: file descriptor to send to
* \param data: data to send
* \param size: size of data to send
* \return: size of data sent, or -1 on error.
*/
virtual I32 sendProtocol(const U8* const data, const U32 size) = 0;
virtual I32 sendProtocol(NATIVE_INT_TYPE fd, const U8* const data, const U32 size) = 0;

/**
* \brief Protocol specific implementation of recv. Called directly with error handling from recv.
* \param fd: file descriptor to recv from
* \param data: data pointer to fill
* \param size: size of data buffer
* \return: size of data received, or -1 on error.
*/
virtual I32 recvProtocol( U8* const data, const U32 size) = 0;
virtual I32 recvProtocol(NATIVE_INT_TYPE fd, U8* const data, const U32 size) = 0;

Os::Mutex m_lock;
NATIVE_INT_TYPE m_fd;
U32 m_timeoutSeconds;
U32 m_timeoutMicroseconds;
U16 m_port; //!< IP address port used
bool m_open; //!< Have we successfully opened
bool m_started; //!< Have we successfully started the socket
char m_hostname[SOCKET_MAX_HOSTNAME_SIZE]; //!< Hostname to supply
};
} // namespace Drv
Expand Down
Loading
Loading