refactor: readability changes to sslclient and wsclient (#1210)

Author: braindigitalis

include/dpp/sslclient.h

@@ -239,10 +239,10 @@ class DPP_EXPORT ssl_client
 
 	/**
 	 * @brief Write to the output buffer.
-	 * @param data Data to be written to the buffer
+	 * @param data Data to be written to the buffer.
 	 * @note The data may not be written immediately and may be written at a later time to the socket.
 	 */
-	virtual void write(const std::string &data);
+	virtual void write(const std::string_view data);
 
 	/**
 	 * @brief Close socket connection

include/dpp/wsclient.h

@@ -23,8 +23,6 @@
 #include <dpp/export.h>
 #include <string>
 #include <map>
-#include <vector>
-#include <variant>
 #include <dpp/sslclient.h>
 
 namespace dpp {
@@ -37,7 +35,7 @@ enum websocket_protocol_t : uint8_t {
 	 * @brief JSON data, text, UTF-8 character set
 	 */
 	ws_json = 0,
-	
+
 	/**
 	 * @brief Erlang Term Format (ETF) binary protocol
 	 */
@@ -68,32 +66,32 @@ enum ws_opcode : uint8_t {
 	/**
 	 * @brief Continuation.
 	 */
-        OP_CONTINUATION = 0x00,
+	OP_CONTINUATION = 0x00,
 
 	/**
 	 * @brief Text frame.
 	 */
-        OP_TEXT = 0x01,
+	OP_TEXT = 0x01,
 
 	/**
 	 * @brief Binary frame.
 	 */
-        OP_BINARY = 0x02,
+	OP_BINARY = 0x02,
 
 	/**
 	 * @brief Close notification with close code.
 	 */
-        OP_CLOSE = 0x08,
+	OP_CLOSE = 0x08,
 
 	/**
 	 * @brief Low level ping.
 	 */
-        OP_PING = 0x09,
+	OP_PING = 0x09,
 
 	/**
 	 * @brief Low level pong.
 	 */
-        OP_PONG = 0x0a
+	OP_PONG = 0x0a
 };
 
 /**
@@ -130,7 +128,7 @@ class DPP_EXPORT websocket_client : public ssl_client {
 	 * @param buffer The buffer to operate on. Will modify the string removing completed items from the head of the queue
 	 * @return true if a complete header has been received
 	 */
-	bool parseheader(std::string &buffer);
+	bool parseheader(std::string& buffer);
 
 	/**
 	 * @brief Unpack a frame and pass completed frames up the stack.
@@ -139,7 +137,7 @@ class DPP_EXPORT websocket_client : public ssl_client {
 	 * @param first True if is the first element (reserved for future use)
 	 * @return true if a complete frame has been received
 	 */
-	bool unpack(std::string &buffer, uint32_t offset, bool first = true);
+	bool unpack(std::string& buffer, uint32_t offset, bool first = true);
 
 	/**
 	 * @brief Fill a header for outbound messages
@@ -151,11 +149,10 @@ class DPP_EXPORT websocket_client : public ssl_client {
 	size_t fill_header(unsigned char* outbuf, size_t sendlength, ws_opcode opcode);
 
 	/**
-	 * @brief Handle ping and pong requests.
-	 * @param ping True if this is a ping, false if it is a pong 
-	 * @param payload The ping payload, to be returned as-is for a ping
+	 * @brief Handle ping requests.
+	 * @param payload The ping payload, to be returned as-is for a pong
 	 */
-	void handle_ping_pong(bool ping, const std::string &payload);
+	void handle_ping(const std::string& payload);
 
 protected:
 
@@ -168,7 +165,7 @@ class DPP_EXPORT websocket_client : public ssl_client {
 	 * @brief Get websocket state
 	 * @return websocket state
 	 */
-	ws_state get_state();
+	[[nodiscard]] ws_state get_state() const;
 
 public:
 
@@ -181,41 +178,41 @@ class DPP_EXPORT websocket_client : public ssl_client {
 	 * @note Voice websockets only support OP_TEXT, and other websockets must be
 	 * OP_BINARY if you are going to send ETF.
 	 */
-        websocket_client(const std::string &hostname, const std::string &port = "443", const std::string &urlpath = "", ws_opcode opcode = OP_BINARY);
+	websocket_client(const std::string& hostname, const std::string& port = "443", const std::string& urlpath = "", ws_opcode opcode = OP_BINARY);
 
 	/**
 	 * @brief Destroy the websocket client object
 	 */
-        virtual ~websocket_client() = default;
+	virtual ~websocket_client() = default;
 
 	/**
 	 * @brief Write to websocket. Encapsulates data in frames if the status is CONNECTED.
 	 * @param data The data to send.
 	 */
-        virtual void write(const std::string &data);
+	virtual void write(const std::string_view data);
 
 	/**
 	 * @brief Processes incoming frames from the SSL socket input buffer.
 	 * @param buffer The buffer contents. Can modify this value removing the head elements when processed.
 	 */
-        virtual bool handle_buffer(std::string &buffer);
+	virtual bool handle_buffer(std::string& buffer);
 
 	/**
 	 * @brief Close websocket
 	 */
-        virtual void close();
+	virtual void close();
 
 	/**
 	 * @brief Receives raw frame content only without headers
-	 * 
+	 *
 	 * @param buffer The buffer contents
 	 * @return True if the frame was successfully handled. False if no valid frame is in the buffer.
 	 */
-	virtual bool handle_frame(const std::string &buffer);
+	virtual bool handle_frame(const std::string& buffer);
 
 	/**
 	 * @brief Called upon error frame.
-	 * 
+	 *
 	 * @param errorcode The error code from the websocket server
 	 */
 	virtual void error(uint32_t errorcode);

src/dpp/sslclient.cpp

@@ -73,7 +73,7 @@
 #include <dpp/dns.h>
 
 /* Maximum allowed time in milliseconds for socket read/write timeouts and connect() */
-#define SOCKET_OP_TIMEOUT 5000
+constexpr uint16_t SOCKET_OP_TIMEOUT{5000};
 
 namespace dpp {
 
@@ -123,10 +123,10 @@ thread_local std::unordered_map<std::string, keepalive_cache_t> keepalives;
  * SSL_read in non-blocking mode will only read 16k at a time. There's no point in a bigger buffer as
  * it'd go unused.
  */
-#define DPP_BUFSIZE 16 * 1024
+constexpr uint16_t DPP_BUFSIZE{16 * 1024};
 
 /* Represents a failed socket system call, e.g. connect() failure */
-const int ERROR_STATUS = -1;
+constexpr int ERROR_STATUS{-1};
 
 bool close_socket(dpp::socket sfd)
 {
@@ -176,8 +176,8 @@ bool set_nonblocking(dpp::socket sockfd, bool non_blocking)
  */
 int connect_with_timeout(dpp::socket sockfd, const struct sockaddr *addr, socklen_t addrlen, unsigned int timeout_ms) {
 #ifdef __APPLE__
-		/* Unreliable on OSX right now */
-		return (::connect(sockfd, addr, addrlen));
+	/* Unreliable on OSX right now */
+	return (::connect(sockfd, addr, addrlen));
 #else
 	if (!set_nonblocking(sockfd, true)) {
 		throw dpp::connection_exception(err_nonblocking_failure, "Can't switch socket to non-blocking mode!");
@@ -197,25 +197,26 @@ int connect_with_timeout(dpp::socket sockfd, const struct sockaddr *addr, sockle
 #endif
 	if (rc == -1 && err != EWOULDBLOCK && err != EINPROGRESS) {
 		throw connection_exception(err_connect_failure, strerror(errno));
-	} else {
-		/* Set a deadline timestamp 'timeout' ms from now */
-		double deadline = utility::time_f() + (timeout_ms / 1000.0);
-		do {
-			rc = -1;
-			if (utility::time_f() >= deadline) {
-				throw connection_exception(err_connection_timed_out, "Connection timed out");
-			}
-			pollfd pfd = {};
-			pfd.fd = sockfd;
-			pfd.events = POLLOUT;
-			int r = ::poll(&pfd, 1, 10);
-			if (r > 0 && pfd.revents & POLLOUT) {
-				rc = 0;
-			} else if (r != 0 || pfd.revents & POLLERR) {
-				throw connection_exception(err_connection_timed_out, strerror(errno));
-			}
-		} while (rc == -1);
 	}
+
+	/* Set a deadline timestamp 'timeout' ms from now */
+	double deadline = utility::time_f() + (timeout_ms / 1000.0);
+
+	do {
+		if (utility::time_f() >= deadline) {
+			throw connection_exception(err_connection_timed_out, "Connection timed out");
+		}
+		pollfd pfd = {};
+		pfd.fd = sockfd;
+		pfd.events = POLLOUT;
+		const int r = ::poll(&pfd, 1, 10);
+		if (r > 0 && pfd.revents & POLLOUT) {
+			rc = 0;
+		} else if (r != 0 || pfd.revents & POLLERR) {
+			throw connection_exception(err_connection_timed_out, strerror(errno));
+		}
+	} while (rc == -1);
+
 	if (!set_nonblocking(sockfd, false)) {
 		throw connection_exception(err_nonblocking_failure, "Can't switch socket to blocking mode!");
 	}
@@ -226,7 +227,7 @@ int connect_with_timeout(dpp::socket sockfd, const struct sockaddr *addr, sockle
 #ifndef _WIN32
 void set_signal_handler(int signal)
 {
-	struct sigaction sa;
+	struct sigaction sa{};
 	sigaction(signal, nullptr, &sa);
 	if (sa.sa_flags == 0 && sa.sa_handler == nullptr) {
 		sa = {};
@@ -378,7 +379,7 @@ void ssl_client::connect()
 	}
 }
 
-void ssl_client::write(const std::string &data)
+void ssl_client::write(const std::string_view data)
 {
 	/* If we are in nonblocking mode, append to the buffer,
 	 * otherwise just use SSL_write directly. The only time we
@@ -388,16 +389,17 @@ void ssl_client::write(const std::string &data)
 	 */
 	if (nonblocking) {
 		obuffer += data;
+		return;
+	}
+
+	const int data_length = static_cast<int>(data.length());
+	if (plaintext) {
+		if (sfd == INVALID_SOCKET || ::send(sfd, data.data(), data_length, 0) != data_length) {
+			throw dpp::connection_exception(err_write, "write() failed");
+		}
 	} else {
-		const int data_length = (int)data.length();
-		if (plaintext) {
-			if (sfd == INVALID_SOCKET || ::send(sfd, data.data(), data_length, 0) != data_length) {
-				throw dpp::connection_exception(err_write, "write() failed");
-			}
-		} else {
-			if (SSL_write(ssl->ssl, data.data(), data_length) != data_length) {
-				throw dpp::connection_exception(err_ssl_write, "SSL_write() failed");
-			}
+		if (SSL_write(ssl->ssl, data.data(), data_length) != data_length) {
+			throw dpp::connection_exception(err_ssl_write, "SSL_write() failed");
 		}
 	}
 }
@@ -502,16 +504,17 @@ void ssl_client::read_loop()
 					read_blocked_on_write = false;
 					read_blocked = false;
 					r = (int) ::recv(sfd, server_to_client_buffer, DPP_BUFSIZE, 0);
+
 					if (r <= 0) {
 						/* error or EOF */
 						return;
-					} else {
-						buffer.append(server_to_client_buffer, r);
-						if (!this->handle_buffer(buffer)) {
-							return;
-						}
-						bytes_in += r;
 					}
+
+					buffer.append(server_to_client_buffer, r);
+					if (!this->handle_buffer(buffer)) {
+						return;
+					}
+					bytes_in += r;
 				} else {
 					do {
 						read_blocked_on_write = false;
@@ -577,14 +580,14 @@ void ssl_client::read_loop()
 					if (r < 0) {
 						/* Write error */
 						return;
-					} else {
-						client_to_server_length -= r;
-						client_to_server_offset += r;
-						bytes_out += r;
 					}
+
+					client_to_server_length -= r;
+					client_to_server_offset += r;
+					bytes_out += r;
 				} else {
 					r = SSL_write(ssl->ssl, client_to_server_buffer + client_to_server_offset, (int)client_to_server_length);
-					
+
 					switch(SSL_get_error(ssl->ssl,r)) {
 						/* We wrote something */
 						case SSL_ERROR_NONE: