# C++ network stream

So, I'm new to socket programming. I wanted to get started learning, and start making stuff, but was really surprised to find out there was no standard C++ network library yet. I'd have to look for a third party library, or make my own.

A third party library search turned up some solutions. Boost::Asio seemed to be the best. However, I wanted something that worked with iostream, similarly to fstream, but for networks. Boost::Asio didn't seem to really support that. Nor did any other library I found. Such a library might exist, but I didn't find it. So I decided to write one.

This, of course, meant I had to learn about making custom iostreams and about networking, 2 things at once. However, I dove in head first and decided to just take it all on. I don't recommend that, but I was up for the challenge.

Overall, I'm quite happy with the result. As far as I can tell, it works perfectly with iostreams, and has an interface similar to fstream, with networking tweaks. Okay, enough introduction.

The design of my Network Streams (as I have called them) is simple. There is a base class NetworkStream which inherits from std::iostream. This class handles all the stream style reading and writing. Most everything else is built off this.

Next is the Client class, which inherits from NetworkStream and handles connecting to a server. Once the connection is set, you can start using it like a regular stream.

Then there is a Server. This basically handles all the listening and has an accept() function that returns an individual connection. Server does not inherit from anything, since it merely serves as the matchmaker.

Finally, there is the ServerSocket class, which basically represents a single connection to a client. This is returned by accept(). The idea being that you have to accept a connection, and then start writing to that connection. Once you have this connection, you can write to it like any old stream.

That's basically all the main classes. There are some custom exceptions thrown in there, but that generally is it.

One thing I should mention about my design: I tried to make it as platform-independent as possible. That sounds crazy, given that socket writing is platform dependent by nature, but fortunately I think I was able to stuff all the platform dependent stuff in its own namespace/file. My thinking was originally that I could write all code platform independent except for some platform-dependent macros. This became messy, as all the macros eventually became spread all over the place, and then I needed to reuse some of them in different files, so there were multiple copies, and such. In the end, I reduced everything down to a simple file that has some inline functions (since macros are generally bad). It made the design much simpler, and, I think, platform independent except for that file.

Things I would like to get reviewed:

• Style. Make sure my code is up to par style-wise.
• Platform independency: I know I tried to make it platform independent. However, trying and doing are two different things. I'm only on windows, so I haven't even tried porting to another platform yet. How easy do you think this is to port? Is there something that probably won't port well to a new platform, specifically linux?
• Thread Safety: I'm on VS2010. As far as I'm aware, VS2010 doesn't have #include <thread> or anything else that works with it like <atomic>. I looked into multi-threading in VS2010, and it doesn't look easy. Thus, I haven't even tried making sure this works well with multi-threading. To do that, I've decided I just need a new compiler that is fully C++11 compliant. In the mean time, I'd like someone with more experience to gauge of how thread-safe this code is, since threaded servers are used in the real world. iostream is supposedly thread-safe, so I'd imagine anything that inherits from it already has some safety built in, but I'm not going to assume that. How safe is this code?
• Error reporting: I think my current error reporting is adequate, but not great. How can this be improved?
• Efficiency: Specifically, NetworkStream uses a read size of 30 for its buffer. Ultimately, I'd like to make it adjustable. Even then, I'd need a default, though. So, I'll ask: what is a good reading buffer size for a stream like this? I've heard everything from 255 to like 1028. What is your recommendation?
• API design: How can this design be improved? Is there anything I should add that most network APIs would support that is missing from here? I want to add UDP, and maybe RAW sockets in the future, but besides that, is there anything that stands out as missing?

Overall, I think it's not bad, for a first pass at least. It does what it's supposed to. It could be more powerful and more flexible, but I think that's a project for the future.

One final thing about the nomenclature before I show my code: "ComputerBytez" is the name of my blog on which I hope to post this stream some day. I'm just putting it in its own namespace since I doubt it will conflict with anything. Plus, once my blog expands (as I'm still really new), having everything encased in a ComputerBytez namespace will just make life easier for readers.

"NetBytez" is just what I decided to call this little project, named after my blog with the stylized "bytez".

Okay, now finally, my code if you're still here:

NetworkStream.h:

#pragma once

#include <iostream>
#include "Platform.h"

// so, this stream runs the basic reading and writing from the network,
// platform independently (except where specified in Platform.h), hopefully

namespace ComputerBytez {
namespace Socket {

// Special thanks to Cay S. Horstmann, who without this post: http://www.horstmann.com/cpp/iostreams.html
// I wouldn't be able to make any of this

class NetworkStream : public std::iostream {
public:

// constructors
NetworkStream(Platform::Socket socket);
virtual ~NetworkStream();

// other functions

// see if we are connected
bool Connected();

// class network streambuff
// should handle the actual writing
class NetworkBuff : public std::streambuf {
public:
NetworkBuff(Platform::Socket s);

void setSocket (Platform::Socket s);
Platform::Socket getSocket ();

// to detect connection
void setConnected(bool c);
bool getConnected();
protected:
// also, shoutout to PanGalactic from http://www.cplusplus.com/forum/general/38408/
// that post helped understand what this function is supposed to do
virtual std::streamsize xsputn(const char* buffer, std::streamsize size);

// Same shoutout as above
virtual std::streambuf::int_type underflow();

private:
Platform::Socket socket;
std::string inputbuffer; // a buffer for the input data
std::string outputbuffer; // a buffer for the output data
bool connected;
}; // end NetworkStream::NetworkBuf

protected:
// prevent copying
NetworkStream(const NetworkStream&);
void operator=(const NetworkStream&);

// internal function to set the state of connected if we need to
void setConnected(bool c);

// internal functions to manipulate our socket.
void setSocket(Platform::Socket socket);
Platform::Socket getSocket();
}; // end network stream

};
}; // end namespace computerbytez


NetworkStream.cpp:

#include "NetworkStream.h"
#include "Exceptions.h"
#include "Platform.h"

// Platform independent except for everything in Platform.h

namespace ComputerBytez {
namespace Socket {

// class network streambuff
// should handle the actual writing

//constructor
NetworkStream::NetworkBuff::NetworkBuff(Platform::Socket s) {
socket = s;
connected = true; // assume we are connected until told otherwise. If we get disconnected, we can log it.
}

// socket setter
void NetworkStream::NetworkBuff::setSocket(Platform::Socket s) {
socket = s;
}

// socket getter
Platform::Socket NetworkStream::NetworkBuff::getSocket() {
return socket;
}

// to detect connection
void NetworkStream::NetworkBuff::setConnected(bool c) {
connected = c;
}
bool NetworkStream::NetworkBuff::getConnected() {
return connected;
}

std::streamsize NetworkStream::NetworkBuff::xsputn(const char* buffer, std::streamsize size) {
// well, let's send the data
int result = Platform::Send(socket,buffer,static_cast<int>(size));

// set up the buffer:
outputbuffer = buffer;

// if that didn't work, throw an error
if(result == Platform::States::SocketError) {
if (Platform::LastError() == Platform::States::Disconnect) {
connected = false;
}
throw(Exceptions::SocketIOError("send"));// Platform::LastError());
}
// NOTE: I realized after I wrote this that this throw may be useless,
// since I think iostream catches any errors thrown at this level, but
// just in case

// set up the pointer
if(outputbuffer.size()==0) {
// do nothing. Probably throw an error in the future
//setp(&outputbuffer[0],&outputbuffer[0],&outputbuffer[0]);
} else {
setp(&outputbuffer[0],&outputbuffer[0],&outputbuffer[outputbuffer.size()-1]);
}
// now return the size
return size;
}

// Shoutout to http://www.voidcn.com/article/p-vjnlygmc-gy.html where I found out
// how to do this proper
std::streambuf::int_type NetworkStream::NetworkBuff::underflow() {

// first, check to make sure the buffer is not exausted:
if(gptr() < egptr()) {
return traits_type::to_int_type(*gptr());
}

// clear the buffer
inputbuffer.clear();
inputbuffer.resize(readsize+1); // +1 is to give our selves room to keep from crashing. Not a good solution, but it works, so...

// return the end of file if we read no bytes
return traits_type::eof();
}

// do nothing right now. Throw an error maybe. Maybe return eof. Perhaps this
// object should be smart enough to hold its own error state that is more useful
// so we can detect things like disconnect. For now, we're just gonna return eof
connected = false;
return traits_type::eof();
}
// set the pointers for the buffer...

// finally, let's return the data type
return traits_type::to_int_type(*gptr());

}

// network stream constructors/destructor
NetworkStream::NetworkStream(Platform::Socket socket) : std::iostream(new NetworkBuff(socket)), std::ios(0) {}
NetworkStream::~NetworkStream() {delete rdbuf();}

void NetworkStream::setSocket(Platform::Socket socket) {
static_cast<NetworkBuff *>(rdbuf())->setSocket(socket);
}

Platform::Socket NetworkStream::getSocket() {
return static_cast<NetworkBuff *>(rdbuf())->getSocket();
}

void NetworkStream::setConnected(bool c) {
static_cast<NetworkBuff *>(rdbuf())->setConnected(c);
}

bool NetworkStream::Connected() {
return static_cast<NetworkBuff *>(rdbuf())->getConnected();
}
}; // end namespace Socket
}; // end namespace ComputerBytez


Platform.h:

/* Basically a way to put all the platform dependent stuff in one place.
* My hope is that everything that is platform dependent makes its way into this module somehow
* That way, porting to a new platform is as simple as adding #ifdefs to this file and/or its cpp
*/
#pragma once
// first, the includes
#ifdef _WIN32
// includes
#include <WinSock2.h>
#include <Windows.h>
#include <WS2tcpip.h>

// and, of course, we need to link:
#pragma comment(lib,"Ws2_32.lib")
#else
#error "Unknown target/platform. You must be using this on a platform it's not built for. Please use on a different platform or consider contributing to extend this"
#endif

namespace ComputerBytez {
namespace Socket {
namespace Platform {
// now the implementation.

// Since I decided to make these guys inline (thoughts on that?), it didn't make sense to
// necessarily create prototypes for them since I have to have the implementation in this header anyway. Also, I'm not
// sure that the prototypes are platform-independent, so I can't have these outside a platform-dependent block (I think)
// So, I decided to just make 1 block and put the implementations next to the prototypes. I do recognize, now that I think
// about it, that it might be nice to still have the prototypes, even if they are platform dependent all in one place
// So, I'll just ask: does anyone have any input on which is best?

#ifdef _WIN32

typedef SOCKET Socket;

namespace States {
extern long Disconnect;
extern int SocketError;
extern unsigned int InvalidSocket;
}; // end namespace states

namespace Protocol {
extern int TCP;
};

// init and cleanup
inline void Init() {
if(result != 0) throw ("Winsock startup failed");
}

inline void Cleanup() {
WSACleanup();
}

inline int _stdcall Send(Socket sock, const char * buf, int len) {
return send(sock,buf,len,0);
}

inline int _stdcall Recv(Socket sock, char* buf, int len) {
return recv(sock,buf,len,0);

}

// creating a socket
inline Socket _stdcall CreateSocket(int af, int type, int protocol) {
return socket(af,type,protocol);
}

// closing the socket
inline int _stdcall CloseSocket(Socket s) {
return closesocket(s);
}

// client stuff
inline Info GetClientHints(int protocol) {
Info hints;
// first, let's zero out the struct
ZeroMemory(&hints, sizeof(hints));

// now, initialize the data we're planning on using
hints.ai_family = AF_UNSPEC; // for now
// detect TCP vs UDP
if(protocol == Protocol::TCP) {
hints.ai_socktype = SOCK_STREAM;
}
// and of course, the protocol will be the same
hints.ai_protocol = protocol;

return hints;
}

inline int _stdcall Connect(Socket sock, const sockaddr* name, int namelen) {
return connect(sock,name,namelen);
}

// server stuff
inline Socket _stdcall Accept(Socket s) {
return accept(s, NULL, NULL);
}

inline int _stdcall Listen(Socket s, int back=SOMAXCONN) {
return listen(s,back);
}

inline int _stdcall Bind(Socket s, const sockaddr* name, int namelen) {
return bind(s,name,namelen);
}

inline Info GetServerHints(int protocol) {
Info hints;
// first, let's zero out the struct
ZeroMemory(&hints, sizeof(hints));

// now, initialize the data we're planning on using
hints.ai_family = AF_INET; // for now

// detect TCP vs UDP
if(protocol == Platform::Protocol::TCP) {
hints.ai_socktype = SOCK_STREAM;
}
// and of course, the protocol will be the same
hints.ai_protocol = protocol;
hints.ai_flags = AI_PASSIVE;

return hints;
}

inline INT _stdcall GetInfo(PCSTR NodeName,PCSTR ServiceName,Info* hints,Info** result) {
}

inline void _stdcall FreeInfo(Info* i) {
}

// get error
inline int _stdcall LastError() {
return WSAGetLastError();
}

#else
#error "Unknown target/platform. You must be using this on a platform it's not built for. Please use on a different platform or consider contributing to extend this"
#endif

}; // end namespace Platform
}; // end namespace Socket
}; // end namespace ComputerBytez


Platform.cpp

/* Basically a way to put all the platform dependent stuff in one place.
* My hope is that everything that is platform dependent makes its way into this module somehow
* That way, porting to a new platform is as simple as adding #ifdefs to this file and/or its cpp
*/
#include "Platform.h"

namespace ComputerBytez {
namespace Socket {
namespace Platform {

#ifdef _WIN32

namespace States {
long Disconnect = WSAENOTCONN;
int SocketError = SOCKET_ERROR;
unsigned int InvalidSocket = INVALID_SOCKET;
}; // end namespace states

namespace Protocol {
int TCP = IPPROTO_TCP;
};
#endif

}; // end namespace Platform
}; // end namespace Socket
}; // end namespace ComputerBytez


Client.h

#include "NetworkStream.h"
#include "Protocol.h"

namespace ComputerBytez {
namespace Socket {

class ClientSocket : public NetworkStream {
public:
void Connect(std::string addr, std::string p, int protocol = Protocol::TCP);

void disconnect();

ClientSocket(std::string addr, std::string por, int pro = Protocol::TCP);

~ClientSocket();

private:
std::string port;

}; // end ClientSocket

}; // end namespace socket
}; // end namespace computerbytez


Client.cpp

#include "Client.h"
#include "Exceptions.h"

namespace ComputerBytez {
namespace Socket {
void ClientSocket::Connect(std::string addr, std::string p, int protocol) {
port = p;
Platform::Socket sock = Platform::States::InvalidSocket; // a tmp sock
int connectresult;

// so, some setup vars
Platform::Info *result = NULL; // a result
Platform::Info hints = Platform::GetClientHints(protocol); // and an initial struct
Platform::Info * ptr = NULL; // a loop variable

// try to get host and port
connectresult = Platform::GetInfo(ServerAddress.c_str(), port.c_str(), &hints, &result);

// if we failed, throw an error
if(connectresult != 0) throw(Exceptions::NoRouteToHost());

// now, let's attempt to connect
for(ptr = result; ptr != NULL; ptr = ptr->ai_next) {
// create a socket
sock = Platform::CreateSocket(ptr->ai_family,ptr->ai_socktype,ptr->ai_protocol);
if(sock == Platform::States::InvalidSocket) throw(Exceptions::SocketCreationError());

// now the actual actual connection attempt

// if we were unsuccessful
if(connectresult == Platform::States::SocketError) {
Platform::CloseSocket(sock);
sock = Platform::States::InvalidSocket;
continue;
}
break;
}

// free some data we created
Platform::FreeInfo(result);

// if the connection was unsuccessful, throw another error
if(sock == Platform::States::InvalidSocket) throw (Exceptions::ConnectionFailed());

// if we've made it here, congratulations! We've connected. one final thing before we go, set up the socket so we can read/write
NetworkStream::setSocket(sock);

// and know that our connection is sucessful
setConnected(true);

}

void ClientSocket::disconnect() {
if(Connected() && NetworkStream::getSocket() != Platform::States::InvalidSocket) {
Platform::CloseSocket(NetworkStream::getSocket());
// and of course, we're disconnected, so...
setConnected(false);
}
}

ClientSocket::ClientSocket(std::string addr, std::string por, int pro) : NetworkStream(0) {
setConnected(false);

}

ClientSocket::~ClientSocket() {disconnect();}

}; // end namespace socket
}; // end namespace computerbytez


Protocol.h

#pragma once
// Originally designed before Platform::Protocol. It was to initially to do Platform::Protocol's job,
// and hold the platform dependent identification of the protocols.

// After making Platform::Protocol, I liked that everything platform related was in one place, yet I also thought Socket::Protocol
// was more understandable name wise. Plus, I had already used Socket::Protocol in code, and since I didn't want the main level to
// ever see the Platform Namespace unless necessary anyway, I decided to keep both. I moved the implelmentation to Platform::Protocol, and
// basically made this another name for it I suppose I could use namespace Platform::Protocol here instead. That might work. Thoughts?

namespace ComputerBytez {
namespace Socket {
namespace Protocol {
extern int TCP;
}; // end namespace Protocol
}; // end namespace Socket
}; // end namespace ComputerBytez



Protocol.cpp

#include "Protocol.h"
#include "Platform.h"

// Platform independent except for everything in Platform.h

namespace ComputerBytez {
namespace Socket {
namespace Protocol {
int TCP = Platform::Protocol::TCP;
}; // end namespace Protocol
}; // end namespace Socket
}; // end namespace ComputerBytez


Server.h

#include "NetworkStream.h"
#include "ServerSocket.h"
#include "Protocol.h"

namespace ComputerBytez {
namespace Socket {
class Server {
private:
// linda, linda, listen to me, listen to me!
Platform::Socket ListenSocket; // the socket we're listening on
std::string port;
bool listening;

// prevent copying
// if anyone wants to chime in on how I can allow copying, let me know
// for now, I'm going to prevent it, so the listen socket doesn't get closed
// accidently due to copying and destructors executing when I don't want them
Server(const Server& other);
void operator=(Server& other);

public:
// contructors and destructors
Server();
Server(std::string p, int protocol = Protocol::TCP);

bool isListening() const;

void StartListen(std::string p, int protocol = Protocol::TCP);

void StopListen();

~Server();

ServerSocket AcceptConnection();

};

}; // end namespace socket
}; // end namespace computerbytez



Server.cpp

#include "Server.h"
#include "Exceptions.h"

namespace ComputerBytez {
namespace Socket {

bool Server::isListening() const { return listening; }

void Server::StartListen(std::string p, int protocol) {
port = p;
int connectresult;

Platform::Info *result = 0; // a result
Platform::Info hints = Platform::GetServerHints(protocol); // and an initial struct

// try to get host and port
connectresult = Platform::GetInfo(NULL, port.c_str(), &hints, &result);

// if we failed, throw an error
if(connectresult != 0) throw(Exceptions::NoRouteToHost());

ListenSocket = Platform::CreateSocket(result->ai_family,result->ai_socktype,result->ai_protocol);

if(ListenSocket == Platform::States::InvalidSocket) {
Platform::FreeInfo(result);
throw(Exceptions::SocketCreationError());
}

// now the bind attempt

// if we were unsuccessful
if(connectresult == Platform::States::SocketError) {
Platform::FreeInfo(result);
Platform::CloseSocket(ListenSocket);
throw(Exceptions::BindFailed());
}

Platform::FreeInfo(result);

// listen time
connectresult = Platform::Listen(ListenSocket);

// if we were unsuccessful
if(connectresult == Platform::States::SocketError) {
Platform::CloseSocket(ListenSocket);
throw(Exceptions::ListenFailed());
}

listening = true;
}

void Server::StopListen() {
// only stop listening if we have a valid socket
if(ListenSocket != Platform::States::InvalidSocket) {
Platform::CloseSocket(ListenSocket);
// now invalidate the socket
ListenSocket = Platform::States::InvalidSocket;

}
// regardless, we are not listening
listening = false;
}

// contructors and destructors
Server::Server() {
ListenSocket = Platform::States::InvalidSocket;
listening = false;
}

Server::Server(std::string p, int protocol) {
ListenSocket = Platform::States::InvalidSocket;
listening = false;
StartListen(p,protocol);

}

Server::~Server() {
// stop listening
StopListen();
}

// This is platform independent
ServerSocket Server::AcceptConnection() {
Platform::Socket s = Platform::States::InvalidSocket;

s = Platform::Accept(ListenSocket);

if(s == Platform::States::InvalidSocket) {
Platform::CloseSocket(ListenSocket);
throw (Platform::LastError());
//throw ("Invalid accept");
}
return ServerSocket(s);
}

}; // end namespace socket
}; // end namespace computerbytez



ServerSocket.h

#include "NetworkStream.h"

// Platform independent except for everything in Platform.h

// this class is so small, I'm not sure if it is worth putting into its own cpp.
// I don't want it to conflict if it's accident;y included multiple times, though, so I may have to
// #pragma once could do the trick. Thoughts?

namespace ComputerBytez {
namespace Socket {
class ServerSocket : public NetworkStream {
public:
ServerSocket(Platform::Socket s) : NetworkStream(s) {

}

~ServerSocket() {
if(NetworkStream::getSocket() != Platform::States::InvalidSocket) {
Platform::CloseSocket(NetworkStream::getSocket());
}
}
};

};
}; // end namespace computerbytez


Exceptions.h

#include <exception>
#include <string>

// Platform independent except for everything in Platform.h

#pragma once
// client exceptions
namespace ComputerBytez {
namespace Socket {
namespace Exceptions {
class NoRouteToHost : public std::exception {
public:
virtual const char* what() {
return "Could not resolve host or port";
}
};

class ConnectionFailed : public std::exception {
public:
virtual const char* what() {
return "Unable to connect to host";
}
};

class BindFailed : public std::exception {
public:
virtual const char* what() {
return "Bind failed";
}
};

class ListenFailed : public std::exception {
public:
virtual const char* what() {
return "Listen failed";
}
};

class SocketCreationError : public std::exception {
public:
virtual const char* what() {
return "Socket creation failed";
}
};

// io errors
class SocketIOError : public std::exception {
private:
std::string type;
public:
SocketIOError(std::string t) : type(t) {}

virtual const char* what() {
return ("Socket encouneted a " + type + "error from the socket").c_str();
}
};

}; // end namespace Exceptions
}; // end namespace Socket
}; // end namespace ComputerBytez



NetBytez.h

#include "Server.h"
#include "Client.h"

// Platform independent except for what's in Platform.h

namespace ComputerBytez {
namespace Socket {
// some intialization functions

void Init ();

void Cleanup();
}; // end namespace Socket
}; // end namespace ComputerBytez

namespace NetBytez {
// NetBytez is a shortcut for the computerbytez socket namespace
using namespace ComputerBytez::Socket;
};


NetBytez.cpp

#include "NetBytez.h"
#include "Platform.h"

// Platform independent except for what's in Platform.h

namespace ComputerBytez {
namespace Socket {
// some intialization functions

void Init () {
Platform::Init();
}

void Cleanup() {
Platform::Cleanup();
}

}; // end namespace Socket
}; // end namespace ComputerBytez


Now some testing code:

TestServer.cpp

#include "NetBytez.h"
#include <string>

int main() {
std::string input = "";

NetBytez::Init();

try {
NetBytez::Server server("3500", NetBytez::Protocol::TCP);
//server.StartListen("3500", NetBytez::Protocol::TCP);

if(server.isListening()) {
std::cout << "Listening on port 3500" << "\n";
}

auto conn = server.AcceptConnection();
std::cout << "Accepted connection on port 3500" << "\n";
//  system("pause");

while(conn.Connected()) {
input.clear();

// read input from the connection
getline(conn, input);

std::cout << "Recieved data. Echoing..." << "\n";
std::cout << "Input: \"" << input << "\"\n";

// echo it back
conn << input + '\n';

//      std::cout << "Input: \"" << input << "\"\n";
}

} catch (char * exception) {
std::cout << "Error! \"" << exception << "\"\n";
} catch (int exception) {
std::cout << "Error code: \"" << exception << "\"\n";
}

NetBytez::Cleanup();

system("pause");

return 0;
}


TestClient.cpp

#include <iostream>
#include <string>

#include "NetBytez.h"
using namespace std;

int main() {
NetBytez::Init();

NetBytez::ClientSocket client("127.0.0.1","3500");
std::string input;

while(true) {
input.clear();
getline(cin,input);
client << input + '\n';
//      input = "";
getline(client, input);

cout << "Got response \"" << input << "\"\n";

}

NetBytez::Cleanup();

return 0;
}


This is a minimal client and server that simply echo data back and forth to make sure that the streams work as intended.

Anyway, after posting this, it made me realize just how much code I have. If anyone has reduction suggestions, that would be appreciated too!

• If you can upgrade to at least VS 2015, VS 2010 doesn't even fully support C++11. Jun 20, 2019 at 12:23
• Yeah. It supports enough to do most C++11 things. Like I said, I'm gonna get a new compiler eventually Jun 20, 2019 at 13:18

# Naming things

There are several ways in which you can improve the way you name things. For example, don't do unnecessary abbreviations. Why NetworkBuff instead of NetworkBuffer? If you do want to abbreviate, make sure you do it the same way as other things do. For example, as you've already seen, the standard library provides std::streambuf, so NetworkBuf might be a better choice.

Also avoid repeating yourself. For example, NetworkStream::NetworkBuff has Network in it twice, and similarly there is Socket::Platform::Socket, Socket::ClientSocket and so on. You use namespaces a lot, maybe some, like Socket, are just not necessary. Or alternatively, rename Socket to Network, and rename NetworkStream to Stream.

Also try to be consistent. Consider these two member functions of ClientSocket:

void Connect(std::string addr, std::string p, int protocol = Protocol::TCP);
ClientSocket(std::string addr, std::string por, int pro = Protocol::TCP);


Why is the port named por in the constructor, but p in Connect()? Why protocol and pro? Again, don't needlessly abbreviate, as it is hard to see from just p that you mean port, and having both por and pro is confusing and a potential source of mistakes.

Also be consistent with the formatting of names. You have some member variables using PascalCase, others use flatcase.

# Use enums where appropriate

Use enum or even better, enum class, where appropriate. For example, instead of:

namespace Protocol {
int TCP = IPPROTO_TCP;
};


Write:

enum class Protocol: int {
TCP = IPPROTO_TCP,
};


This is will allow the compiler to be much stricter in checking that you don't accidentily pass a value that is not one of the valid enum choices. The only drawback is that sometimes you have to explicitly cast such a variable back to the underlying integer type.

# Pass large objects by const reference where appropriate

C++ normally passes parameters by value, which means it has to make copies. For small parameters like ints, that's totally fine, but for something like std::string it would mean it will allocate memory for a new string, and then copy the string. That is inefficient, which is why you should consider passing them via const references instead. For example:

void ClientSocket::Connect(const std::string &addr, const std::string &port, Protocol protocol) {
...
}


# Unnecessary and dangerous casts

Why is readsize explicitly cast to an int in NetworkStream::NetworkBuff::underflow(), when it is already an int? That cast is not necessary. Then there is ptr->ai_addrlen being cast to an int in ClientSocket::Connect(). A socklen_t might be larger than an int, in which case this cast could silently truncate the size, or perhaps even turn it into something negative. While it is probably fine here (after all, who would expect addrlen to be larger than a few tens of bytes?), casts like these are subtle sources of security bugs. Try to find a way to avoid these casts, and if you really need a cast, consider always casting to a larger type if possible.

I tried to compile it in Visual Studio 2019. I got several warnings and 1 error.

I would recommend the following change to the SocketIOError class because the what function caused a warning:

class SocketIOError : public std::exception {
private:
std::string type;
std::string msg;
public:
SocketIOError(std::string t) : type(t) {}

virtual const char* what()
{
msg = "Socket encouneted a " + type + "error from the socket";
return msg.c_str();
}
};


I would also recommend to change the type of InvalidSocket from unsigned int to Socket. That way it doesn't generate a warning when compiled for 64 bits.

In the function Server::AcceptConnection() I get an delete function error (E1776) on the statement

return ServerSocket(s);


At the moment I don't know what to do about that but I think it is caused by the NetworkStream class that prevents copying.

• It was clearly mentioned that the code was written using Visual Studio 2010. That limits it to C++11. May 30, 2021 at 14:35
• Also, copying an exception should not ever throw. May 30, 2021 at 15:04