A simple NAT library

Question

I've created a simple NAT library, with four essential functions:

1. Find the UNPN device (the router in my case).
2. Get the external IP address.
3. Add a port forwarding entry.
4. Delete a port forwarding entry.

Currently, updating an entry in the NAT table will change only the internal client ip address or the internal port, so to update an external port you have to delete it first then insert a new one.

TODO:

1. Add other functionality (listing mapping entries, number of entries ...).
2. Add support for other platforms.
3. more testing if possible.

An example is included; just un-comment the part that you want to test. The library compiled and tested against Visual Studio 2008.

Any suggestions: optimization, coding style, bugs.

main.c:

#pragma comment(lib, "ws2_32.lib")
#include <winsock2.h>
#include <ws2tcpip.h>
#include <windows.h>
#include <stdio.h>
#if (_MSC_VER >= 1500)
 #include <Strsafe.h>
#endif
#define MALLOC(x) HeapAlloc(GetProcessHeap(), 0, (x))
#define FREE(x) HeapFree(GetProcessHeap(), 0, (x))
#define TCP_PROTOCOL 0
#define UDP_PROTOCOL 1
#define INVALID_ARGS 402
#define ACTION_FAILED 501
#define ENTRY_MAPPING_NOTEXIST 714
#define INVALID_REMOTEHOST_IP 715
#define INVALID_REMOTEHOST_PORT 716 
#define ENTRY_CONFLICT 718
#define SAME_PORT_REQUIRED 724
#define ONLY_PERMANENT_LEASE 725
#define OWILDCARD_IP_REQUIRED 726
#define OWILDCARD_PORT_REQUIRED 727
int boradcastDiscovery(char *upnpDeviceIp);
int getExternalpAddress(char *upnpDeviceIp, char *localIp, char *externalIpAddress);
int addPortForwardEntry(char *upnpDeviceIp, char *localIp, int externalPort, int internalPort, int protocol, char *internalp, char *entryDescription);
int deletePortForwardEntry(char *upnpDeviceIp, char *localIp, int externalPort, int protocol);
int main() {
 char buf[512],
 ipAddress[16];
 int errorCode = 0;
 /*ZeroMemory(ipAddress, 16);
 boradcastDiscovery(ipAddress);
 printf("UPNP device IP: %s\n", ipAddress);*/
 /*ZeroMemory(ipAddress, 16);
 errorCode = getExternalpAddress("192.168.1.1", "192.168.1.4", ipAddress);
 switch(errorCode) {
 case INVALID_ARGS:
 printf("Invalid query arg's.\n");
 break;
 case ACTION_FAILED:
 printf("Query failed to get external IP.\n");
 break;
 default:
 printf("External IP address: %s\n", ipAddress);
 break;
 }*/
 /*ZeroMemory(ipAddress, 16);
 errorCode = addPortForwardEntry("192.168.1.1", "192.168.1.4", 9000, 4000, TCP_PROTOCOL, "192.168.1.7", "TEST FROM LIBRARY");
 switch(errorCode) {
 case INVALID_ARGS:
 printf("Invalid query arg's.\n");
 break;
 case ACTION_FAILED:
 printf("Query failed to add entry mapping.\n");
 break;
 case INVALID_REMOTEHOST_IP:
 printf("Invalid IP address.\n");
 break;
 case INVALID_REMOTEHOST_PORT:
 printf("Invalid Port number.\n");
 break;
 case ENTRY_CONFLICT:
 printf("Such entry already exist.\n");
 break;
 case SAME_PORT_REQUIRED:
 printf("External and internal port must be the same.\n");
 break;
 case ONLY_PERMANENT_LEASE:
 printf("External and internal port must be the same.\n");
 break;
 case OWILDCARD_IP_REQUIRED:
 printf("External and internal port must be the same.\n");
 break;
 case OWILDCARD_PORT_REQUIRED:
 printf("External and internal port must be the same.\n");
 break;
 default:
 printf("Port mapping entry added sussccefully.\n");
 break;
 }*/
 /*ZeroMemory(ipAddress, 16);
 errorCode = deletePortForwardEntry("192.168.1.1", "192.168.1.2", 5000, TCP_PROTOCOL);
 switch(errorCode) {
 case INVALID_ARGS:
 printf("Invalid query arg's.\n");
 break;
 case ACTION_FAILED:
 printf("Query failed to add entry mapping.\n");
 break;
 case ENTRY_MAPPING_NOTEXIST:
 printf("Entry mapping doesn't exist.\n");
 break;
 default:
 printf("Port mapping entry deleted sussccefully.\n");
 break;
 }*/
 while(1)
 ;
 return 0;
}

UpNp discover:

int boradcastDiscovery(char *upnpDeviceIp) {
 char *searchIGDevice = "M-SEARCH * HTTP/1.1\r\nHost:239.255.255.250:1900\r\nST:urn:schemas-upnp-org:device:InternetGatewayDevice:1\r\nMan:\"ssdp:discover\"\r\nMX:3\r\n\r\n",
 buf[512];
 int result = -1;
 WSADATA wsaData;
 struct sockaddr_in upnpControl, 
 broadcast_addr;
 result = WSAStartup(MAKEWORD(2, 2), &wsaData); 
 if(result != 0)
 return WSAGetLastError();
 SOCKET sock = INVALID_SOCKET;
 sock = socket(AF_INET, SOCK_DGRAM, 0);
 if (sock == INVALID_SOCKET)
 return WSAGetLastError();
 if(setsockopt(sock, SOL_SOCKET, SO_BROADCAST, searchIGDevice, sizeof(searchIGDevice)) == SOCKET_ERROR)
 return WSAGetLastError();
 upnpControl.sin_family = AF_INET;
 upnpControl.sin_port = htons(0);
 upnpControl.sin_addr.s_addr = INADDR_ANY;
 if (bind(sock, (sockaddr*)&upnpControl, sizeof(upnpControl)) == SOCKET_ERROR)
 return WSAGetLastError();
 broadcast_addr.sin_family = AF_INET;
 broadcast_addr.sin_port = htons(1900);
 broadcast_addr.sin_addr.s_addr = inet_addr("239.255.255.250");
 if(sendto(sock, searchIGDevice, lstrlen(searchIGDevice)+1, 0, (sockaddr *)&broadcast_addr, sizeof(broadcast_addr)) == SOCKET_ERROR)
 return WSAGetLastError();
 int bcLen = sizeof(broadcast_addr);
 ZeroMemory(buf, 512);
 if(recvfrom(sock, buf, 512, 0, (sockaddr *)&broadcast_addr, &bcLen) < 0)
 return WSAGetLastError();
 else {
 closesocket(sock);
 WSACleanup();
 if(strstr(buf, "device:InternetGatewayDevice")) {
 int i = 0;
 char *deviceIp = strstr(buf, "http://") + 7;
 while(*deviceIp != ':') {
 upnpDeviceIp[i] = *deviceIp;
 *deviceIp++;
 ++i;
 }
 return 0;
 }
 else
 return -1;
 }
}

Get external IP address:

int getExternalpAddress(char *upnpDeviceIp, char *localIp, char *externalIpAddress) {
 WSADATA wsaData;
 struct sockaddr_in upnpControl, 
 upnpDevice;
 int i = 0,
 j = 0,
 result = -1,
 bodyLen = 0;
 char c, 
 *tmp = NULL,
 *pLen = NULL,
 *bodyResponse = NULL, // a pointer to HTTP body response
 pBodyLen[5], 
 responseHeader[512],
 getExternalIpRequest[622]; 
 int reuseAddress = 1;
 // A SOAP request to get external IP address
 ZeroMemory(getExternalIpRequest, 622);
 StringCbPrintf(getExternalIpRequest, 622,
 "POST /UD/?3 HTTP/1.1\r\n"
 "Content-Type: text/xml; charset=\"utf-8\"\r\n" 
 "SOAPAction: \"urn:schemas-upnp-org:service:WANIPConnection:1#GetExternalIPAddress\"\r\n"
 "User-Agent: Mozilla/4.0 (compatible; UPnP/1.0; Windows 9x)\r\n"
 "Host: %s\r\n"
 "Content-Length: 303\r\n"
 "Connection: Close\r\n"
 "Cache-Control: no-cache\r\n"
 "Pragma: no-cache\r\n\r\n"
 "<?xml version=\"1.0\"?>"
 "<SOAP-ENV:Envelope xmlns:SOAP-ENV=\"http://schemas.xmlsoap.org/soap/envelope/\" SOAP-ENV:encodingStyle=\"http://schemas.xmlsoap.org/soap/encoding/\">"
 "<SOAP-ENV:Body>"
 "<m:GetExternalIPAddress xmlns:m=\"urn:schemas-upnp-org:service:WANIPConnection:1\"/>"
 "</SOAP-ENV:Body>"
 "</SOAP-ENV:Envelope>\r\n\r\n", upnpDeviceIp);
 result = WSAStartup(MAKEWORD(2, 2), &wsaData);
 if(result != 0)
 return WSAGetLastError();
 SOCKET sock = INVALID_SOCKET;
 sock = socket(AF_INET, SOCK_STREAM, 0);
 if(sock == INVALID_SOCKET)
 return WSAGetLastError();
 if(setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (char *)&reuseAddress, sizeof(reuseAddress)) == SOCKET_ERROR)
 return WSAGetLastError();
 ZeroMemory(&upnpControl, sizeof(upnpControl));
 upnpControl.sin_family = AF_INET;
 if(localIp == NULL)
 upnpControl.sin_addr.s_addr = INADDR_ANY;
 else
 upnpControl.sin_addr.s_addr = inet_addr(localIp);
 ZeroMemory(&upnpDevice, sizeof(upnpDevice));
 upnpDevice.sin_family = AF_INET;
 upnpDevice.sin_port = htons(80);
 upnpDevice.sin_addr.s_addr = inet_addr(upnpDeviceIp);
 if(connect(sock, (struct sockaddr *)&upnpDevice, sizeof(struct sockaddr)) == SOCKET_ERROR)
 return WSAGetLastError();
 if(send(sock, getExternalIpRequest, lstrlen(getExternalIpRequest), 0) == SOCKET_ERROR)
 return WSAGetLastError();
 ZeroMemory(responseHeader, 512);
 while(recv(sock, &c, 1, 0) > 0) {
 responseHeader[i] = c;
 // We got the http header, extract the body length from it
 if(strstr(responseHeader, "\r\n\r\n")) {
 // Move the pointer to the first digit
 pLen = strstr(responseHeader, "Content-Length: ") + 16;
 ZeroMemory(pBodyLen, 5);
 // Get the body length
 while(*pLen != '\r') {
 pBodyLen[j] = *pLen;
 *pLen++;
 ++j;
 }
 bodyLen = atoi(pBodyLen);
 j = 0;
 bodyResponse = (char *)MALLOC(bodyLen);
 while(recv(sock, &c, 1, 0) > 0) {
 bodyResponse[j] = c;
 ++j;
 if(j == bodyLen) { // We got the HTTP body
 closesocket(sock);
 }
 }
 }
 ++i;
 }
 i = 0;
 tmp = strstr(bodyResponse, "<NewExternalIPAddress>") + 22;
 ZeroMemory(externalIpAddress, 16);
 while(*tmp != '<') {
 externalIpAddress[i] = *tmp;
 *tmp++;
 ++i;
 }
 FREE(bodyResponse);
 WSACleanup();
 return 0;
}

Add port forward entry:

int addPortForwardEntry(char *upnpDeviceIp, char *localIp, int externalPort, int internalPort, 
 int protocol, char *internalp, char *entryDescription) {
 WSADATA wsaData;
 struct sockaddr_in upnpControl, upnpDevice;
 int i = 0,
 j = 0,
 result = -1,
 bodyLen = 0,
 numErrorCode = 0;
 char c, 
 *pLen = NULL,
 *proto = NULL,
 *bodyResponse = NULL,
 pBodyLen[5],
 responseHeader[2000],
 addForwardEntryRequest[1200],
 addForwardEntryRequestHeader[1500]; 
 proto = (char *)MALLOC(4);
 ZeroMemory(proto, 4);
 if(protocol == TCP_PROTOCOL)
 StringCbPrintf(proto, 4, "TCP");
 else
 StringCbPrintf(proto, 4, "UDP");
 // A SOAP request to insert a port forwarding entry
 ZeroMemory(addForwardEntryRequest, 1200);
 ZeroMemory(addForwardEntryRequestHeader, 1500);
 StringCbPrintf(addForwardEntryRequest, 1200,
 "<?xml version=\"1.0\"?>"
 "<SOAP-ENV:Envelope xmlns:SOAP-ENV=\"http://schemas.xmlsoap.org/soap/envelope/\" SOAP-ENV:encodingStyle=\"http://schemas.xmlsoap.org/soap/encoding/\">"
 "<SOAP-ENV:Body>"
 "<m:AddPortMapping xmlns:m=\"urn:schemas-upnp-org:service:WANIPConnection:1\">"
 "<NewRemoteHost>"
 ""
 "</NewRemoteHost>"
 "<NewExternalPort>"
 "%d"
 "</NewExternalPort>"
 "<NewProtocol>"
 "%s"
 "</NewProtocol>"
 "<NewInternalPort>"
 "%d"
 "</NewInternalPort>"
 "<NewInternalClient>"
 "%s"
 "</NewInternalClient>"
 "<NewEnabled>"
 "1"
 "</NewEnabled>"
 "<NewPortMappingDescription>"
 "%s"
 "</NewPortMappingDescription>"
 "<NewLeaseDuration>"
 "0"
 "</NewLeaseDuration>"
 "</m:AddPortMapping>"
 "</SOAP-ENV:Body>"
 "</SOAP-ENV:Envelope>\r\n\r\n", externalPort, proto, internalPort, internalp, entryDescription);
 StringCbPrintf(addForwardEntryRequestHeader, 1500, 
 "POST /UD/?3 HTTP/1.1\r\n"
 "Content-Type: text/xml; charset=\"utf-8\"\r\n" 
 "SOAPAction: \"urn:schemas-upnp-org:service:WANIPConnection:1#AddPortMapping\"\r\n"
 "User-Agent: Mozilla/4.0 (compatible; UPnP/1.0; Windows 9x)\r\n"
 "Host: %s\r\n"
 "Content-Length: %d\r\n"
 "Connection: Close\r\n"
 "Cache-Control: no-cache\r\n"
 "Pragma: no-cache\r\n\r\n", upnpDeviceIp, lstrlen(addForwardEntryRequest));
 StringCchCat(addForwardEntryRequestHeader, 1500, addForwardEntryRequest);
 result = WSAStartup(MAKEWORD(2, 2), &wsaData);
 if(result != 0)
 return WSAGetLastError();
 SOCKET sock = INVALID_SOCKET;
 sock = socket(AF_INET, SOCK_STREAM, 0);
 if(sock == INVALID_SOCKET)
 return WSAGetLastError();
 ZeroMemory(&upnpControl, sizeof(upnpControl));
 upnpControl.sin_family = AF_INET;
 if(localIp == NULL)
 upnpControl.sin_addr.s_addr = INADDR_ANY;
 else
 upnpControl.sin_addr.s_addr = inet_addr(localIp);
 upnpControl.sin_port = 0;
 ZeroMemory(&upnpDevice, sizeof(upnpDevice));
 upnpDevice.sin_family = AF_INET;
 upnpDevice.sin_port = htons(80);
 upnpDevice.sin_addr.s_addr = inet_addr(upnpDeviceIp);
 if(connect(sock, (struct sockaddr *)&upnpDevice, sizeof(upnpDevice)) == SOCKET_ERROR)
 return WSAGetLastError();
 if(send(sock, addForwardEntryRequestHeader, lstrlen(addForwardEntryRequestHeader), 0) == SOCKET_ERROR)
 return WSAGetLastError();
 ZeroMemory(responseHeader, 2000);
 while(recv(sock, &c, 1, 0) > 0) {
 responseHeader[i] = c;
 if(strstr(responseHeader, "\r\n\r\n")) {
 // Move the pointer to the first digit
 pLen = strstr(responseHeader, "Content-Length: ") + 16;
 ZeroMemory(pBodyLen, 5);
 // Get the body length
 while(*pLen != '\n') {
 pBodyLen[j] = *pLen;
 *pLen++;
 ++j;
 }
 bodyLen = atoi(pBodyLen); // body length
 j = 0;
 bodyResponse = (char *)MALLOC(bodyLen);
 ZeroMemory(bodyResponse, bodyLen);
 while(recv(sock, &c, 1, 0) > 0) {
 bodyResponse[j] = c;
 ++j;
 if(j == bodyLen) {
 closesocket(sock);
 }
 }
 }
 ++i;
 }
 // Check if uPnp reject our entry
 i = 0;
 if(strstr(bodyResponse, "<errorCode>")) {
 char errorCode[3],
 *tmp = strstr(bodyResponse, "<errorCode>") + 11;
 while(*tmp != '<') {
 errorCode[i] = *tmp;
 *tmp++;
 ++i;
 }
 FREE(bodyResponse);
 numErrorCode = atoi(errorCode);
 return numErrorCode;
 }
 // Check the entry that we insert if is exist in the mapping table
 ZeroMemory(addForwardEntryRequest, 1200);
 ZeroMemory(addForwardEntryRequestHeader, 1500);
 StringCbPrintf(addForwardEntryRequest, 1200, 
 "<SOAP-ENV:Envelope xmlns:SOAP-ENV=\"http://schemas.xmlsoap.org/soap/envelope/\" SOAP-ENV:encodingStyle=\"http://schemas.xmlsoap.org/soap/encoding/\">\r\n"
 "<SOAP-ENV:Body>\r\n"
 "<m:GetSpecificPortMappingEntry xmlns:m=\"urn:schemas-upnp-org:service:WANIPConnection:1\">\r\n"
 "<NewRemoteHost/>\r\n"
 "<NewExternalPort>%d</NewExternalPort>\r\n"
 "<NewProtocol>%s</NewProtocol>\r\n"
 "</m:GetSpecificPortMappingEntry>\r\n"
 "</SOAP-ENV:Body>\r\n"
 "</SOAP-ENV:Envelope>\r\n\r\n", externalPort, proto);
 StringCbPrintf(addForwardEntryRequestHeader, 1500, 
 "POST /UD/?3 HTTP/1.1\r\n"
 "Content-Type: text/xml; charset=\"utf-8\"\r\n" 
 "SOAPAction: \"urn:schemas-upnp-org:service:WANIPConnection:1#GetSpecificPortMappingEntry\"\r\n"
 "User-Agent: Mozilla/4.0 (compatible; UPnP/1.0; Windows 9x)\r\n"
 "Host: %s\r\n"
 "Content-Length: %d\r\n"
 "Connection: Close\r\n"
 "Cache-Control: no-cache\r\n"
 "Pragma: no-cache\r\n\r\n", upnpDeviceIp, lstrlen(addForwardEntryRequest));
 StringCchCat(addForwardEntryRequestHeader, 1500, addForwardEntryRequest);
 sock = INVALID_SOCKET;
 sock = socket(AF_INET, SOCK_STREAM, 0);
 if(sock == INVALID_SOCKET)
 return WSAGetLastError();
 if(connect(sock, (struct sockaddr *)&upnpDevice, sizeof(upnpDevice)) == SOCKET_ERROR)
 return WSAGetLastError();
 if(send(sock, addForwardEntryRequestHeader, lstrlen(addForwardEntryRequestHeader), 0) == SOCKET_ERROR)
 return WSAGetLastError();
 i = 0;
 ZeroMemory(responseHeader, 2000);
 while(recv(sock, &c, 1, 0) > 0) {
 responseHeader[i] = c;
 if(strstr(responseHeader, "\r\n\r\n")) {
 // Move the pointer to the first digit
 j = 0;
 pLen = strstr(responseHeader, "Content-Length: ") + 16;
 ZeroMemory(pBodyLen, 5);
 // Get the body length
 while(*pLen != '\n') {
 pBodyLen[j] = *pLen;
 *pLen++;
 ++j;
 }
 bodyLen = atoi(pBodyLen);
 j = 0;
 bodyResponse = (char *)MALLOC(bodyLen);
 ZeroMemory(bodyResponse, bodyLen);
 while(recv(sock, &c, 1, 0) > 0) {
 bodyResponse[j] = c;
 ++j;
 if(j == bodyLen) {
 closesocket(sock);
 WSACleanup();
 }
 }
 }
 ++i;
 }
 // Check if the request rejected
 i = 0;
 if(strstr(bodyResponse, "<errorCode>")) {
 char errorCode[3],
 *tmp = strstr(bodyResponse, "<errorCode>") + 11;
 while(*tmp != '<') {
 errorCode[i] = *tmp;
 *tmp++;
 ++i;
 }
 FREE(bodyResponse);
 numErrorCode = atoi(errorCode);
 return numErrorCode;
 }
 return 0;
}

Delete port forward entry:

int deletePortForwardEntry(char *upnpDeviceIp, char *localIp, int externalPort, int protocol) {
 WSADATA wsaData;
 struct sockaddr_in upnpControl, upnpDevice;
 int i = 0,
 j = 0,
 result = -1,
 bodyLen = 0,
 numErrorCode = 0;
 char c, 
 *pLen = NULL,
 *proto = NULL,
 *bodyResponse = NULL,
 pBodyLen[5],
 responseHeader[700],
 deleteForwardEntryRequest[700],
 deleteForwardEntryRequestHeader[1000]; 
 proto = (char *)MALLOC(4);
 ZeroMemory(proto, 4);
 if(protocol == TCP_PROTOCOL)
 StringCbPrintf(proto, 4, "TCP");
 else
 StringCbPrintf(proto, 4, "UDP");
 // A SOAP request to delete a mapping entry
 ZeroMemory(deleteForwardEntryRequest, 700);
 ZeroMemory(deleteForwardEntryRequestHeader, 1000);
 StringCbPrintf(deleteForwardEntryRequest, 1200,
 "<?xml version=\"1.0\"?>"
 "<SOAP-ENV:Envelope xmlns:SOAP-ENV=\"http://schemas.xmlsoap.org/soap/envelope/\" SOAP-ENV:encodingStyle=\"http://schemas.xmlsoap.org/soap/encoding/\">"
 "<SOAP-ENV:Body>"
 "<m:DeletePortMapping xmlns:m=\"urn:schemas-upnp-org:service:WANIPConnection:1\">"
 "<NewRemoteHost>"
 ""
 "</NewRemoteHost>"
 "<NewExternalPort>"
 "%d"
 "</NewExternalPort>"
 "<NewProtocol>"
 "%s"
 "</NewProtocol>"
 "</m:DeletePortMapping>"
 "</SOAP-ENV:Body>"
 "</SOAP-ENV:Envelope>\r\n\r\n", externalPort, proto);
 StringCbPrintf(deleteForwardEntryRequestHeader, 1500, 
 "POST /UD/?3 HTTP/1.1\r\n"
 "Content-Type: text/xml; charset=\"utf-8\"\r\n" 
 "SOAPAction: \"urn:schemas-upnp-org:service:WANIPConnection:1#DeletePortMapping\"\r\n"
 "User-Agent: Mozilla/4.0 (compatible; UPnP/1.0; Windows 9x)\r\n"
 "Host: %s\r\n"
 "Content-Length: %d\r\n"
 "Connection: Keep-Alive\r\n"
 "Cache-Control: no-cache\r\n"
 "Pragma: no-cache\r\n\r\n", upnpDeviceIp, lstrlen(deleteForwardEntryRequest));
 StringCchCat(deleteForwardEntryRequestHeader, 1500, deleteForwardEntryRequest);
 result = WSAStartup(MAKEWORD(2, 2), &wsaData);
 if(result != 0)
 return WSAGetLastError();
 SOCKET sock = INVALID_SOCKET;
 sock = socket(AF_INET, SOCK_STREAM, 0);
 if(sock == INVALID_SOCKET)
 return WSAGetLastError();
 ZeroMemory(&upnpControl, sizeof(upnpControl));
 upnpControl.sin_family = AF_INET;
 if(localIp == NULL)
 upnpControl.sin_addr.s_addr = INADDR_ANY;
 else
 upnpControl.sin_addr.s_addr = inet_addr(localIp);
 upnpControl.sin_port = 0;
 ZeroMemory(&upnpDevice, sizeof(upnpDevice));
 upnpDevice.sin_family = AF_INET;
 upnpDevice.sin_port = htons(80);
 upnpDevice.sin_addr.s_addr = inet_addr(upnpDeviceIp);
 if(connect(sock, (struct sockaddr *)&upnpDevice, sizeof(upnpDevice)) == SOCKET_ERROR)
 return WSAGetLastError();
 if(send(sock, deleteForwardEntryRequestHeader, lstrlen(deleteForwardEntryRequestHeader), 0) == SOCKET_ERROR)
 return WSAGetLastError();
 ZeroMemory(responseHeader, 700);
 while(recv(sock, &c, 1, 0) > 0) {
 responseHeader[i] = c;
 if(strstr(responseHeader, "\r\n\r\n")) {
 // Move the pointer to the first digit
 j = 0;
 pLen = strstr(responseHeader, "Content-Length: ") + 16;
 ZeroMemory(pBodyLen, 5);
 // Get the body length
 while(*pLen != '\n') {
 pBodyLen[j] = *pLen;
 *pLen++;
 ++j;
 }
 bodyLen = atoi(pBodyLen);
 j = 0;
 bodyResponse = (char *)MALLOC(bodyLen);
 ZeroMemory(bodyResponse, bodyLen);
 while(recv(sock, &c, 1, 0) > 0) {
 bodyResponse[j] = c;
 ++j;
 if(j == bodyLen) {
 closesocket(sock);
 WSACleanup();
 }
 }
 }
 ++i;
 }
 // Check f our request rejected
 i = 0;
 if(strstr(bodyResponse, "<errorCode>")) {
 char errorCode[3],
 *tmp = strstr(bodyResponse, "<errorCode>") + 11;
 while(*tmp != '<') {
 errorCode[i] = *tmp;
 *tmp++;
 ++i;
 }
 FREE(bodyResponse);
 numErrorCode = atoi(errorCode);
 return numErrorCode;
 }
 else
 return 0;
}

• Jamal: I'd recommend splitting this code into separate blocks, or only include what's most necessary for review. Excessively-long code could discourage reviewing.
• SIFE: @Jamal Do you mean splitting code by inserting each function in its own code tag?
• Jamal: Only if this would work as separate questions. Otherwise, you could just have more than one block in this question, each with a short description of what's included.
• William Morris: What's the point of main - it is just a while(1) loop with everything else commented out.

Answer 1

My main comment is that you need to concentrate on avoiding duplication of code and on writing smaller functions. 50 lines or so is the sort of max length I use. You also use long names that are often too long giving the code a very dense appearance and making it difficult ot read. Smaller functions, reduced variable scope and hence shorter name sizes will help here.

In function boradcastDiscovery, the name could be better as the current (misspelt) name doesn't say what the function does. getGateway() perhaps?

In this function we have:

struct sockaddr_in upnpControl,
 broadcast_addr;
SOCKET sock = INVALID_SOCKET;
sock = socket(AF_INET, SOCK_DGRAM, 0);
if (sock == INVALID_SOCKET)
 return WSAGetLastError();
if(setsockopt(sock, SOL_SOCKET, SO_BROADCAST, searchIGDevice, sizeof(searchIGDevice)) == SOCKET_ERROR)
 return WSAGetLastError();
struct sockaddr_in upnpControl;
upnpControl.sin_family = AF_INET;
upnpControl.sin_port = htons(0);
upnpControl.sin_addr.s_addr = INADDR_ANY;
if (bind(sock, (sockaddr*)&upnpControl, sizeof(upnpControl)) == SOCKET_ERROR)
 return WSAGetLastError();

This could easily be extracted to a function:

static int
getBroadcastSocket(char *device, size_t size)
{
 SOCKET sock = socket(AF_INET, SOCK_DGRAM, 0);
 if (sock == INVALID_SOCKET) {
 return -1;
 }
 int status = setsockopt(sock, SOL_SOCKET, SO_BROADCAST, device, size);
 if (status != SOCKET_ERROR) {
 struct sockaddr_in s;
 s.sin_family = AF_INET;
 s.sin_port = htons(0);
 s.sin_addr.s_addr = INADDR_ANY;
 status = bind(sock, (sockaddr*)&s, sizeof s);
 }
 if (status == SOCKET_ERROR) {
 close(sock);
 return -1;
 }
 return sock;
}

Notice that sock is closed on error.

Another example is in code such as this which extracts a field from a string and is repeated numerous times thoughout the whole code:

if(strstr(responseHeader, "\r\n\r\n")) {
 // Move the pointer to the first digit
 pLen = strstr(responseHeader, "Content-Length: ") + 16;
 ZeroMemory(pBodyLen, 5);
 // Get the body length
 while(*pLen != '\r') {
 pBodyLen[j] = *pLen;
 *pLen++;
 ++j;
 }

This should be extracted into a suitable function. Also of note in this code are:

• the use of explicit lengths (16 and 5 here), which is bad practice;
• the lack of checks for target buffer overflow (often difficult);
• the use of variable j which was initialize far away at the start of the function;
• inappropriate loop type - a for would be better.

---

A few other comments:

• The address "239.255.255.250" would be better if extracted to a #define at the top perhaps.

• The code

  char *proto = NULL,
  ...
  proto = (char *)MALLOC(4);
  ZeroMemory(proto, 4);
  if(protocol == TCP_PROTOCOL)
   StringCbPrintf(proto, 4, "TCP");
  else
   StringCbPrintf(proto, 4, "UDP");
  

would be better as

 const char *proto = (protocol == TCP_PROTOCOL) ? "TCP" : "UDP";

• Some of your calls to ZeroMemory often look redundant. The sizes in these calls and in calls to StringCbPrintf are often explicit numbers rather than using sizeof, which would be better.

• The huge xml strings are distracting and would, I think, be better extracted into suitably named functions, passing in the target buffer and the parameters that need to be printed into the strings.

• Variables should be defined close to their first point of use and initialised on use if possible. For example there is no benefit in writing:

  SOCKET sock = INVALID_SOCKET;
  sock = socket(AF_INET, SOCK_STREAM, 0);
  

  when you could write

  SOCKET sock = socket(AF_INET, SOCK_STREAM, 0);
  

• you are using TCP so you don't have to create a complete string before sending it. For example instead of writing

  createHeader(header, ...);
  createRequest(request, ...);
  concatenate(combined, header, request);
  send(combined);
  

  you can instead do:

  createHeader(header, ...);
  send(header);
  createRequest(request, ...);
  send(request);
  

Hopefully you will find something above that is of use :-)