Thread: Weird connection problem.

Hi everyone ! I'm trying to make a console chat application but I have a problem. I tried the program with someone and that person managed to receive the "Connected !" message, but I didn't. We both use Linksys routers and have set the proper parameters to connect to each other. I gave him the correct IP and I'm using the port #1234 for my program. Finally, we both are using Windows XP. Using the following code, why don't I (the listener) receive the "Connected !" message ?

Code:

#include <winsock2.h>
#include <windows.h>
#include <iostream>
#include <string>
#pragma comment(lib, "ws2_32.lib")
#define DEFAULT_PORT                1234
#define BUFFER_SIZE                255
#define PEER_DISCONNECTED    0

SOCKET    ConnectionSocket        = 0;
bool            Connected                = false;

// This function can only be used on recv() and send().
inline bool Success(int ret_value)
{
    return !((ret_value == SOCKET_ERROR) || (ret_value == PEER_DISCONNECTED));
}

DWORD WINAPI ListeningProc(LPVOID params)
{
    sockaddr_in sin;
    ZeroMemory(&sin, sizeof(sin));
    sin.sin_addr.s_addr    = INADDR_ANY;
    sin.sin_family                = AF_INET;
    sin.sin_port                = htons(DEFAULT_PORT);

    SOCKET s = socket(AF_INET, SOCK_STREAM, 0);
    if(s != INVALID_SOCKET)
    {
        if(bind(s, (sockaddr*)&sin, sizeof(sin)) != SOCKET_ERROR)
        {
            listen(s, 10);

            int buf = 0;
            ConnectionSocket = accept(s, (sockaddr*)&sin, &buf);
            if(ConnectionSocket != INVALID_SOCKET)
            {
                std::cout << "Connected !";
                Connected = true;
            }
        }
    }
    return 0;
}

DWORD WINAPI ReceivingProc(LPVOID params)
{
    char Buffer[BUFFER_SIZE];
    unsigned int ByteNb = 0;
    unsigned int Temp = 0;
    while(Connected == true)
    {
        // Receive the number of bytes and then receive the message itself.
        if(Success(recv(ConnectionSocket, (char*)&Buffer, BUFFER_SIZE, 0)))
        {
            if(strcmp(Buffer, "") == 0)
            {
                break;
            }
            ByteNb = atoi(Buffer) * sizeof(char);

            std::cout << "\nAway: "; 
            while(ByteNb > 0)
            {
                ZeroMemory(Buffer, sizeof(Buffer));
                Temp = recv(ConnectionSocket, (char*)&Buffer, BUFFER_SIZE, 0);
                if(Success(Temp))
                {
                    std::cout << Buffer;
                    ByteNb -= Temp;
                }
                else if(Temp == PEER_DISCONNECTED)
                {
                    Connected = false;
                    std::cout << "Peer disconnected...";
                    break;
                }
                std::cout << std::endl;
            }
        }
    }
    return 0;
}

int main()
{
    std::cout << "Win32 console chat application\n"
                        "[localhost] to connect to yourself.\n"
                        "[quit] to quit at any time.\n\n";

    WSADATA WsaData;
    if(WSAStartup(MAKEWORD(2,2), &WsaData) != SOCKET_ERROR)
    {
        HANDLE ListeningThread = CreateThread(0, 0, ListeningProc, 0, 0, 0);
        unsigned long IP = 0;
        std::string DistantIP = "";

        while(Connected == false)
        {
            std::cout << "Please enter distant computer's IP: ";
            std::getline(std::cin, DistantIP);
            if(DistantIP == "[quit]")
            {
                TerminateThread(ListeningThread, 0);
                break;
            }
            IP = inet_addr((DistantIP == "[localhost]") ? "127.0.0.1" : DistantIP.c_str());
            if(IP != INADDR_NONE )
            {
                sockaddr_in sin;
                ZeroMemory(&sin, sizeof(sin));
                sin.sin_addr.s_addr    = IP;
                sin.sin_family                = AF_INET;
                sin.sin_port                = htons(DEFAULT_PORT);

                SOCKET s = socket(AF_INET, SOCK_STREAM, 0);
                if(connect(s, (sockaddr*)&sin, sizeof(sin)) != SOCKET_ERROR)
                {
                    std::cout << "Connected !";
                    Connected = true;
                    ConnectionSocket = s;
                    closesocket(s);
                    TerminateThread(ListeningThread, 0);
                }
                else
                {
                    std::cout << "Failed to connect..." << std::endl;
                }
            }
        }

        if(Connected == true)
        {
            std::string Message;
            while(Connected == true)
            {
                break;
            }
            closesocket(ConnectionSocket);
        }

        WSACleanup();
    }
    std::cout << "Adios amigos";
    std::cin.get();
    return 0;
}

It can't be the problem since my friend got the "Connected !" message on his screen but I didn't.

Edit: I made sure my program wasn't blocked by my firewall before trying.

Your accept line is wrong -

ConnectionSocket = accept(s, (sockaddr*)&sin, &buf);

You need to set buf to sizeof(sin) before you call accept:

int buf = sizeof(sin);
ConnectionSocket = accept(s, (sockaddr*)&sin, &buf);

MSDN is a little confusing here because it says addrlen is an [out] parameter, but then says

The integer referred to by addrlen initially contains the amount of space pointed to by addr. On return it will contain the actual length in bytes of the address returned.

Thank you mates, I will try this ASAP.

Code:

        if(Success(recv(ConnectionSocket, (char*)&Buffer, BUFFER_SIZE, 0)))
        {
            if(strcmp(Buffer, "") == 0)

As a general rule, don't use the addressof operator (&) on an array. Used alone, an array variable will return a pointer to the first element of the array. This is almost always what you want. &array will return a pointer to the array. It is very rare to need this and this is why you are having to use a cast in your code.

Also, you must treat all data received over the network as untrusted. This means that if you wish to use received data as a string, you must make sure you nul terminate it. Also, another reason that received data may not be nul terminated is that data may be split up into multiple recv calls.

Code:

        data_size = recv(ConnectionSocket, Buffer, BUFFER_SIZE - 1, 0);
        if (data_size >= 0)
        {
            Buffer[data_size] = '\0';
            if(strcmp(Buffer, "") == 0)