Review Network Code

closed account (N36fSL3A)
Ok so I just put a wrapper over winsock, and I'm just wondering what you think of this code. Is there any sort of way I can save on ram, or save on CPU cycles? Thanks.

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
 
#ifndef _CFNETWORKING_H_
#define _CFNETWORKING_H_

#include "Core.h"
#include "Error.h"
#include <WinSock2.h>

class CF::Networking::System
{
	public:
		System() {}

		void Init()
		{
			bool success    = false;
			WSADATA wsaData = {};

			success = (WSAStartup(MAKEWORD(2, 2), &wsaData) == NO_ERROR);
		}

		void Cleanup()
		{
			WSACleanup();
		}

		~System()
		{
			Cleanup();
		}

	private:

};

class CF::Networking::Socket
{
	public:
		Socket(unsigned short port)
		{
			Open(port);
		}

		// * Opens a socket on the specified port
		void Open(unsigned short port)
		{
			this->port = port;
			handle     = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
			DWORD nonBlock = 1;

			if(handle < 0)
			{
				throw CF::Error("CF::Networking::Socket::Socket() - Failure getting handle to socket.");
			}

			address.sin_family      = AF_INET;
			address.sin_addr.s_addr = INADDR_ANY;
			address.sin_port        = htons(port);

			if(bind(handle, (const sockaddr*)&address, sizeof(sockaddr_in)) < 0)
			{
				throw CF::Error("CF::Networking::Socket::Socket::() - Failure binding socket.");
			}

			if (ioctlsocket(handle, FIONBIO, &nonBlock) != 0)
			{
				throw CF::Error("CF::Networking::Socket::Socket::() - Failure setting non-binding socket.");
			}
		}

		// * Send a packet over the network
		template<class T>
		void  Send(const char* destAddr, T* packet, size_t numElem = 1)
		{
			if(numElem == 0) numElem = 1;

			size_t sentBytes  = 0;
			sockaddr_in sAddr;

			// The packet casted into a const char pointer
			const char* c_packet = reinterpret_cast<const char*>(packet);

			sAddr.sin_family           = AF_INET;
			sAddr.sin_addr.S_un.S_addr = inet_addr(destAddr);
			sAddr.sin_port = port;

			sentBytes = sendto(handle, c_packet, (int)size, 0, (const sockaddr*)&sAddr, sizeof(sAddr));

			// If the whole packet wasn't sent throw an error
			if(sentBytes != size)
				throw CF::Error("CF::Networking::Socket::Send() - Entire packet was not sent.");
		}

		// * Recieve a packet from a server
		template<class T>
		void Recv(T* buffer, size_t numElem = 1)
		{
			if(numElem == 0) numElem = 1;

			sockaddr_in sAddr;

			sAddr.sin_family           = AF_INET;
			sAddr.sin_addr.S_un.S_addr = inet_addr(destAddr);
			sAddr.sin_port             = port;

			if(recvfrom(handle, (char*)buffer, sizeof(T) * numElem, 0, (sockaddr*)&sAddr, sizeof(sockaddr_in)) == SOCKET_ERROR)
			{
				throw CF::Error("CF::Networking::Socket::Recv() - recvfrom() failed.");
			}
		}

		void Close()
		{
			closesocket(handle);

			handle = 0;
		}

		~Socket()
		{
			Close();
		}

	private:
		CF::UintPtr    handle;
		sockaddr_in    address;
		unsigned short port;
};

#endif//_CFNETWORKING_H_
Last edited on
LB (13399)
Why do you want to save on RAM and CPU cycles? Have you profiled it and found that the code is excessively slow and/or that it takes up a lot of memory?
Stewbond (2827)
Generally, I like it. Here are a few ideas in comments:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
 
#ifndef _CFNETWORKING_H_
#define _CFNETWORKING_H_

#include "Core.h"
#include "Error.h"
#include <WinSock2.h>

class CF::Networking::System
{
    public:
        System() {}

        // Either make this return bool, or don't bother driving success.
        // Another option is to throw something if an error occurs like you do everywhere else
        void Init() 
        {
            bool success    = false;
            WSADATA wsaData = {};

            success = (WSAStartup(MAKEWORD(2, 2), &wsaData) == NO_ERROR);
        }

        void Cleanup()
        {
            WSACleanup();
        }

        ~System()
        {
            Cleanup();
        }

    private:

};

class CF::Networking::Socket
{
    public:
        Socket(unsigned short port)
        {
            Open(port);
        }

        void Open(unsigned short port)
        {
            this->port = port;
            handle     = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
            DWORD nonBlock = 1;

            if(handle < 0) // maybe <= instead?
            {
            // You could use: (__FUNCTION__ "- Failure getting handle to socket.") if your compiler supports it.
                throw CF::Error("CF::Networking::Socket::Socket() - Failure getting handle to socket.");
            }

            address.sin_family      = AF_INET;
            address.sin_addr.s_addr = INADDR_ANY;
            address.sin_port        = htons(port);

            if(bind(handle, (const sockaddr*)&address, sizeof(sockaddr_in)) < 0)
            {
                throw CF::Error("CF::Networking::Socket::Socket::() - Failure binding socket.");
            }

            if (ioctlsocket(handle, FIONBIO, &nonBlock) != 0)
            {
                throw CF::Error("CF::Networking::Socket::Socket::() - Failure setting non-binding socket.");
            }
        }
        
        // Not sure what destArrd should contain based on the type.
        // Maybe you want to make an IPAddress class which can be constructed in a few ways.
        template<class T>
        void  Send(const char* destAddr, T* packet, size_t numElem = 1)
        {
            if(numElem == 0) numElem = 1;

            size_t sentBytes  = 0;
            sockaddr_in sAddr;

            const char* c_packet = reinterpret_cast<const char*>(packet);

            sAddr.sin_family           = AF_INET;
            sAddr.sin_addr.S_un.S_addr = inet_addr(destAddr);
            sAddr.sin_port = port;

            sentBytes = sendto(handle, c_packet, (int)size, 0, (const sockaddr*)&sAddr, sizeof(sAddr));

            if(sentBytes != size)
                throw CF::Error("CF::Networking::Socket::Send() - Entire packet was not sent.");
        }

        template<class T>
        void Recv(T* buffer, size_t numElem = 1)
        {
            if(numElem == 0) numElem = 1;

            sockaddr_in sAddr;

            sAddr.sin_family           = AF_INET;
            sAddr.sin_addr.S_un.S_addr = inet_addr(destAddr);
            sAddr.sin_port             = port;

            if(recvfrom(handle, (char*)buffer, sizeof(T) * numElem, 0, (sockaddr*)&sAddr, sizeof(sockaddr_in)) == SOCKET_ERROR)
            {
                throw CF::Error("CF::Networking::Socket::Recv() - recvfrom() failed.");
            }
        }

        void Close()
        {
            closesocket(handle);

            handle = 0;
        }

        ~Socket()
        {
            Close();
        }

    private:
        CF::UintPtr    handle;
        sockaddr_in    address;
        unsigned short port;
};

#endif//_CFNETWORKING_H_
Stewbond (2827)
For CPU cycles, you could use return values instead of throwing exceptions. Exceptions take some heavy processing.

Const-correctness might be good too to help optimize things

Finally, for this line:
handle = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
You might want to make a virtual protected function for each option. This would let you use this class as a base class. You could overload your protected functions in any child classes to allow for things like TCP (instead of supporting just UDP).
closed account (N36fSL3A)
Well I wrote a rather lengthy reply but it seems as it's not hear anymore because I accidentally switched off my Wi-Fi.

Anyway, @LB
Actually, I don't have any problems regarding ram, but while basically idling with all my subsystems initialized, plus one thing loaded in the 3 main ones, I get ~ 3 MB ram usage...

@Stewbond Thanks for the input, it really helps. I'll add TCP support, but I felt like it was a waste since most games use UDP.
Topic archived. No new replies allowed.