recvcfrom() and sendto() ip address to be used

Question

Actually, I want to create an application in C such that 2 people can chat with each other. Let us assume they know their IP (Actually, I think I am making the mistake here. I get my IPs from www.whatismyip.com).

void recv_data(char *from, unsigned short int Port, char *data, int data_length)
{
                WSADATA wsaData;
                SOCKET RecvSocket;
                sockaddr_in RecvAddr;
                char RecvBuf[data_length];
                sockaddr_in SenderAddr;
                int SenderAddrSize = sizeof (SenderAddr);
                WSAStartup(MAKEWORD(2, 2), &wsaData);
                RecvSocket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
                RecvAddr.sin_family = AF_INET;
                RecvAddr.sin_port = htons(Port);
                   RecvAddr.sin_addr.s_addr = inet_addr(from);
                bind(RecvSocket, (SOCKADDR *) & RecvAddr, sizeof (RecvAddr));
                recvfrom(RecvSocket, RecvBuf, data_length, 0, (SOCKADDR *) & SenderAddr, &SenderAddrSize);
                int i;
            for(i=0;i<=data_length-1;i++)
                *(data+i)=RecvBuf[i];
            WSACleanup();
}

The above is a function to receive what the other person is sending. It works great when "127.0.0.1" is the value of from but when my ip (117.193.52.176) is used, something else appears. Could anyone tell me where I am wrong ?

Answer 1

The address you passing to "bind" is likely wrong. Just use the IP of INADDR_ANY (0) for the call to bind. I suspect 117.193.52.176 is likely your external IP address outside of your home NAT. Your PC's real IP address is 192.168.1.2 or something like that. Type "ipconfig /all" from the command line. In any case, just bind to INADDR_ANY so you don't have to know your real IP address.

Other issues with this code:

1. Not checking return values from socket APIs
2. Don't call WSAStartup and WSACleanup for every recvfrom call. Just call WSAStartup once in your app, and don't worry about calling WSACleanup.
3. I'm not entirely sure if the line "char RecvBuf[data_length];" will compile. (Dynamically length static buffer on the stack? Maybe it's a new compiler feature).
4. Don't create a new socket for every recvfrom call. Create it once and bind to it, then use it for all subsequent send/recv calls.

5.. A more fundamnetal design problem. Unless both you and person you are communicating with are directly connected to the Internet (not NAT and no firewall), sending and receiving UDP packets will be difficult. Read the article on hole-punching here.

In any case, here's a cleaner version of your code:

int g_fWinsockInit = 0;

void initWinsock()
{
    WSADATA wsaData = {};

    if(!g_fWinsockInit)
    {
        WSAStartup(MAKEWORD(2,2), &wsaData);
        g_fWinsockInit = 1;
    }
}

void recv_data(char *from, unsigned short int Port, char *data, int data_length)
{
    SOCKET RecvSocket;
    sockaddr_in RecvAddr = {}; // zero-init, this will implicitly set s_addr to INADDR_ANY (0)

    sockaddr_in SenderAddr = {}; // zero-init
    int SenderAddrSize = sizeof(SendAddr);
    int ret;

    initWinsock();

    RecvSocket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
    if (RecvSocket == INVALID_SOCK)
    {
        printf("Error - socket failed (err = %x)\n", WSAGetLastError());
        return;
    }

    RecvAddr.sin_family = AF_INET;
    RecvAddr.sin_port = htons(Port);

    ret = bind(RecvSocket, (SOCKADDR *) & RecvAddr, sizeof (RecvAddr));
    if (ret < 0)
    {
       printf("bind failed (error = %x)\n", WSAGetLastError());
       return;
    }

    ret = recvfrom(RecvSocket, data, data_length, 0, (SOCKADDR *) &SenderAddr, &SenderAddrSize);

    if (ret < 0)
    {
       printf("recvfrom failed (error = %x)\n", WSAGetLastError());
    }
    else
    {
        printf("received %d bytes\n");
    }

}