TCP/IP Packet Send/Recv problem

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 01:02

I’m using SDL_net to create a simple server and client program in C++. I’m using TCP to send data from the client (sender) to the server (listener).

I know that TCP is a data stream protocol and that packets arrive at their destination complete and in order but with no indication of where one packet ends and another begins. I’ve been trying to work around the problem of getting “merged” or “split” packets on the server.

I’m using a function similar to the sendall() function that Beej uses in his guide, however my packets are still arriving split.

Here’s my sending function:

int sendAll(TCPsocket socket, char *buff, unsigned short len)
{
    unsigned short total = 0;
    unsigned short bytesleft = len;
    int n;

    n = SDLNet_TCP_Send(socket, &len, sizeof(short));

    while (total < len)
    {
        n = SDLNet_TCP_Send(socket, buff, bytesleft);
        if (n == -1) { break; }
        total += n;
        bytesleft -= n;
    }

    return n == -1?-1:0;
}

And my receiving function

int recvAll(TCPsocket socket, char *buff)
{
    unsigned short total = 0;
    unsigned short len = 0;
    int n;

    n = SDLNet_TCP_Recv(socket, &len, sizeof(short));
    unsigned short bytesleft = len;

    while (total < len)
    {
        n = SDLNet_TCP_Recv(socket, buff, bytesleft);
        if (n == -1) { break; }
        total += n;
        bytesleft -= n;
    }

    return n == -1?-1:0;    // Return -1 on failure, 0 on success;
}

I first send/recv an unsigned short denoting the length of the message, and then I make repeated calls to send/recv until at least that many bytes has been received. I then print the output to the screen.

I’m unsure as to why this isn’t working. If I were to send a character string containing “Hello World !”, I get 3 seperate outputs, “Hello”, “World” and “!”.

Can anyone tell me what I’m doing wrong? Or maybe a better (read: easier) way of doing it?

29 Replies

Please log in or register to post a reply.

17ba6d8b7ba3b6d82970a7bbba71a6de
0
vrnunes 102 Aug 27, 2009 at 01:59

Not clear to me. Not working at all, or not working because the server is receiving 3 packets?

I don’t know about SDL_net, but if “len” has less than 512 bytes I bet it’ll be sent in one send() and received with one recv().

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 02:32

That’s what I thought, and I’ve been wracking my brain for days trying to work out why my messages are being sent word by word. Then I just realised, it is because I am getting my messages using “cin >> msg”.

I made a simple program (not related to networking) where I got user input using cin, and only the first word of the message was printed, so I’m guessing this is where I have gone wrong?

I tried changing every instance of “cin >> msg” in my program, to getline(cin, msg) but doing so gives me errors. Are there any other c++ input functions that are used to get a whole line of input?

EDIT: To clarify, it is not working because when i send a character string such as “The Quick Brown Fox”, the server outputs:

Received: The
Received: Quick
Received: Brown
Received: Fox

When it ought to just be receiving “The Quick Brown Fox” as a single output.

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 167 Aug 27, 2009 at 02:48

getline should work. What errors do you receive using that?

6837d514b487de395be51432d9cdd078
0
TheNut 179 Aug 27, 2009 at 03:00

c++ cin breaks on whitespace, so if you enter 3 words that counts as 3 inputs. To manage a single input line use:

char buffer[128];
std::cin.getline(buffer, sizeof(buffer));

You can adjust the size of the buffer as you need.

To manage multiple packets you should devise some sort of application level protocol. My protocol contains a 4 byte header on every message sent that identifies the length of the message. Once the message has been fully received, I deserialize the binary data into XML (typically) or some sort of object for application level processing. Any leftover data gets pushed over for the next message to download.

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 03:13

@Reedbeta

getline should work. What errors do you receive using that?

I get an error because one of SDL_net’s functions expects a pointer to a char as an argument (in this case, the argument is char msg[512]). Using getline(cin, msg) where msg is a pointer to a char won’t work for me, and changing msg from a char* to a string doesn’t bode well with my SDL_net function.

However, if TheNut’s code works and I can use cin.getline() to input data into a character array, I should be able to finally get my code working!

Thanks for the replies, I’m gonna go attemp to fix it ;)

EDIT: It’s working great ;) thanks very much for the help, I can’t believe I’ve spent so long trying so solve a problem that was so simple!

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 167 Aug 27, 2009 at 03:55

You can also make msg a string, use getline(), then call msg.c_str() to convert it to a char pointer to send to your SDL_net function. In fact, I would recommend doing it that way because then you don’t have to worry about overflowing the fixed-size buffer in TheNut’s code.

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 05:15

Thank you for the info Reedbeta. If I use a string and convert it to a char pointer using msg.c_str(), am I still able to enfore a maximum message length?

For instance, maximum message length is 512 and a client sends a string of length 520. I would like for the 8 trailing chars to be cut off.

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 167 Aug 27, 2009 at 05:33

Well, if you really do want a fixed-size buffer, you could go ahead and use TheNut’s version. (It’s safe, in that it won’t read more characters from the stream than the size of the buffer, but will leave any additional characters in the stream to be read the next time.) The string version will read a whole line, however long it happens to be. You can then certainly send only the first 512 characters if you like. Just ensure you clamp the number of bytes to a max of 512 for your send routine.

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 05:42

So assuming msg is a string, something like SDLNet_TCP_Send(socket, msg.c_str(), 512) would only send the first 512 characters and ignore the rest? (Without leaving them in the stream to be sent with the next call to Send()?)

If so then you’re right, it would be better for me to use strings. Thanks!

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 167 Aug 27, 2009 at 06:00

Close, but you should say min(512, msg.size()) rather than just 512. You don’t want it to try to send 512 characters if the string is shorter than that, after all. :)

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 06:19

Thanks again, although now I am a little confused!
So you mean, the line should be SDLNet_TCP_Send(socket, msg.c_str(), min(512, msg.size()))?

EDIT: Should have Googled it first. I understand now that min() returns the lesser of it’s two arguments. In this case either the size of msg, or 512 (whichever is smaller) ?

On the receiving end, I would use something similar to this? :

string msg;

SDLNet_TCP_Recv(socket, msg.c_str(), length);
A8433b04cb41dd57113740b779f61acb
0
Reedbeta 167 Aug 27, 2009 at 06:47

After you receive the length of the string, you should use the resize() method to ensure the string has the right number of characters allocated before you receive the string data itself. But yes, then something like that should work, although you may have to cast away the const-ness of the pointer returned from c_str().

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 07:07

Still a little confused :(

So something like this? :

string msg;
unsigned short len;

// Read first two bytes of data (containing length)
SDLNet_TCP_Recv(socket, len, sizeof(short);

msg.resize(len);

SDLNet_TCP_Recv(socket, msg.c_str(), len);

I’ve probably got it even more wrong now haha, sorry about all the questions.

Also I’m unsure how to “cast away const-ness”, in fact I didn’t even know you could.

EDIT: Again, I really ought to Google before I post. I now know how I can cast away const-ness, but in this situation I am not sure as to why I would need to? Am I not just able to print the received msg.c_str() to the screen by using cout << msg; ?

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 167 Aug 27, 2009 at 16:26

Yes, that code looks right. Casting away constness has nothing to do with printing the message on the screen. I simply meant that c_str() returns a const char *, while SDLNet_TCP_Recv takes a void * (not const). So, to get this to compile you would need to cast away the constness when you pass the pointer returned from c_str() to SDLNet_TCP_Recv. This should be reasonably safe although you’re not really supposed to do it (which is why c_str() returns a pointer to const in the first place).

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 17:31

So as it is I might get a compiler error?
I am unable to test it right now, but would using const_cast within that single function call work? eg:

SDLNet_TCP_Recv(socket, const_cast<char *>(msg.c_str()), len);

Thanks again for the help and sorry for so many questions (and my general lack of understanding!)

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 167 Aug 27, 2009 at 17:37

Yes, the const_cast within the function call is what I had in mind.

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 17:44

Great! Thanks for all your help Reedbeta. Glad I (finally) got that cleared up :D

Might actually be able to rewrite my code to compile without errors now!

Fd80f81596aa1cf809ceb1c2077e190b
0
rouncer 104 Aug 27, 2009 at 17:48

Another happy customer :)

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 21:51

For completeness’ sake I thought I’d post my (now working) client and server code. Thanks again for helping me to get this work!

Server:

/*
    A simple server program
    by Luke Jennings 2009
*/

/********************/

/* Link additional libraries */
#pragma comment(lib, "sdl.lib")
#pragma comment(lib, "sdlmain.lib")
#pragma comment(lib, "sdl_net.lib")

/* Defines */
#define PORT 1234   // The port we are connecting to
#define BUFFER 512  // Size of message buffer
#define MAXSOCKET 10    // Max number of clients to connect

/* Standard headers */
#include <cstdlib>
#include <iostream>
#include <string>

/* SDL headers */
#include "sdl.h"
#include "sdl_net.h"

using namespace std;

/********************/

/** MAIN **/

int main(int argc, char **argv)
{
    // Variables
    const char *host;   // Where we store the host name
        
    IPaddress ipaddress;    // The IP we will connect to
    TCPsocket tcpsock;  // The server socket
    TCPsocket new_tcpsock;  // A temp socket
    
    TCPsocket client[MAXSOCKET];    // An array of sockets for the clients
    
    SDLNet_SocketSet socketset = NULL;  // A set of sockets
    
    int i = 0, j = 0, result = 0;
    unsigned short len = 0;
    string msg;
    
    /********************/
    
    // Initialize SDL & SDL_net
    if (SDL_Init(0) == -1)
    {
        cout << "SDL_Init: " << SDL_GetError() << "\n";
        exit(1);
    }
    if (SDLNet_Init() == -1)
    {
        cout << "SDLNet_Init: " << SDLNet_GetError() << "\n";
        exit(2);
    }
    
    // Create the socket set
    socketset = SDLNet_AllocSocketSet(MAXSOCKET);
    if (socketset == NULL)
    {
        cout << "SDLNet_AllocSocketSet: " << SDLNet_GetError() << "\n";
        exit(3);
    }
    else
    {
        cout << "Max number of clients: " << MAXSOCKET << "\n";
    }
    
    // Initialize client sockets
    for (i = 0; i < MAXSOCKET; i++)
    {
        client[i] = NULL;
    }
    
    // Try to resolve the host
    if (SDLNet_ResolveHost(&ipaddress, NULL, PORT) == -1)
    {
        cout << "SDLNet_ResolveHost: " << SDLNet_GetError() << "\nContinuing...\n";
    }
    
    // Try to resolve the IP
    if ((host = SDLNet_ResolveIP(&ipaddress)) == NULL)
    {
        cout << "SDLNet_ResolveIP: " << SDLNet_GetError() << "\nContinuing...\n";
    }
    else
    {
        cout << "Local name: " << host << "\n";
    }
    
    // State which port we are listening on
    cout << "Port: " << PORT << "\n";
    
    // Try to open the server socket
    tcpsock = SDLNet_TCP_Open(&ipaddress);
    if (!tcpsock)
    {
        cout << "SDLNet_TCP_Open: " << SDLNet_GetError() << "\n";
        exit(4);
    }
    
    // Add the server to the socket set
    SDLNet_TCP_AddSocket(socketset, tcpsock);
    
    cout << "Awaiting clients...\n";
    
    /********************/
        
    /** MAIN LOOP **/
    
    while (1)
    {
        // Check for activity on the socket set
        SDLNet_CheckSockets(socketset, 0);
        
        // If there is activity
        if (SDLNet_SocketReady(tcpsock))
        {
            // Accept the connection, add it to our array and the socket set
            cout << "Client connected.\n";
            
            new_tcpsock = SDLNet_TCP_Accept(tcpsock);
            client[j] = new_tcpsock;
            
            SDLNet_TCP_AddSocket(socketset, client[j]);
            j++;
        }
        
        // Check client sockets for activity
        for (i = 0; i < MAXSOCKET; i++)
        {
            if (SDLNet_SocketReady(client[i]))
            {
                // There is an incoming message
                result = SDLNet_TCP_Recv(client[i], const_cast<char *>(msg.c_str()), BUFFER);
                
                if (result < 0)
                {
                    cout << "Client " << i << " disconnected.\n";
                    client[i] = NULL;
                }
                else
                {
                    cout << "Received: " << msg.c_str() << "\n";
                }
            }
        }
    }
    
    /********************/
        
    // Quit SDL & SDL_net
    SDLNet_Quit();
    SDL_Quit();
        
    return 0;
}

Client:

/*
    A simple client program
    by Luke Jennings 2009
*/

/********************/

/* Link additional libraries */
#pragma comment(lib, "sdl.lib")
#pragma comment(lib, "sdlmain.lib")
#pragma comment(lib, "sdl_net.lib")

/* Defines */
#define PORT 1234   // The port we are connecting to
#define BUFFER 512  // Size of message buffer

/* Standard headers */
#include <cstdlib>
#include <iostream>
#include <string>

/* SDL headers */
#include "sdl.h"
#include "sdl_net.h"

using namespace std;

/********************/

/** MAIN **/

int main(int argc, char **argv)
{
    // Variables
    const char *host;   // Where we store the host name
    
    IPaddress ipaddress;    // The IP we will connect to
    TCPsocket tcpsock;  // The socket to use
    
    string servername;  // The server name
    
    int result = 0;
    unsigned short len = 0;
    string msg;
    
    /********************/
    
    // Initialize SDL & SDL_net
    if (SDL_Init(0) == -1)
    {
        cout << "SDL_Init: " << SDL_GetError() << "\n";
        exit(1);
    }
    if (SDLNet_Init() == -1)
    {
        cout << "SDLNet_Init: " << SDLNet_GetError() << "\n";
        exit(2);
    }
    
    // Get the server name
    cout << "Server Name: ";
    getline(cin, servername);
    cout << "Port: " << PORT << "\n";
    
    // Try to resolve the host
    if (SDLNet_ResolveHost(&ipaddress, servername.c_str(), PORT) == -1)
    {
        cout << "SDLNet_ResolveHost: " << SDLNet_GetError() << "\nContinuing...\n";
    }
    
    // Try to resolve the IP
    if ((host = SDLNet_ResolveIP(&ipaddress)) == NULL)
    {
        cout << "SDLNet_ResolveIP: " << SDLNet_GetError() << "\nContinuing...\n";
    }
    else
    {
        cout << "Connected to host: " << host << "\n";
    }
    
    // Try to open the socket
    tcpsock = SDLNet_TCP_Open(&ipaddress);
    if (!tcpsock)
    {
        cout << "SDLNet_TCP_Open: " << SDLNet_GetError() << "\n";
        exit(3);
    }
    
    /********************/
    
    /** MAIN LOOP **/
    
    while (1)
    {
        // Get user input
        cout << "Send: ";
        getline(cin, msg);
        
        // Calculate the length and send it
        len = (int)(strlen(msg.c_str()));
        result = SDLNet_TCP_Send(tcpsock, msg.c_str(), min(BUFFER, (int)len));
        
        cout << "Sent: (" << len << ") " << msg << "\n";
    }
    
    /********************/
    
    // Quit SDL & SDL_net
    SDLNet_Quit();
    SDL_Quit();
    
    return 0;
}

The only issue I have (which isn’t too much of a problem), is that after receiving data into msg.c_str(), I can only output it as a character array and not as a string. This may become a problem when I try to fix the code that sends messages back to clients. Any ideas?

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 167 Aug 27, 2009 at 21:58

What do you mean by “can only output it as a character array and not as a string”?

You’ll probably need to add the null terminator yourself if you want most C string functions to work on it. To do that, you should make sure to resize() it to one more byte than the message length (BTW, I notice you lost the resize() call in your most recent code; it may appear to work, but you’re not allocating memory properly, and you’re likely to run into an invalid page fault sooner or later), and then store 0 to that last byte (or just fill the whole buffer with zeroes before receiving the message).

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 22:13

I meant that usually I would output a string like so:
cout << string << “\n”;
But that doesn’t work after I use my recv() code (which receives the data as string.c_str()). To output the data I need to use:
cout << string.c_str() << “\n”;

As for the call to resize. I got rid of my custom recv() function (that received the length of data first), just to save time and test the code I already have. Within that function I had a call to resize(). I realise now my mistake, as the server now crashes if it receives a message longer than 21 bytes :( I’ll go put it back in lol.

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 167 Aug 27, 2009 at 22:34

What do you mean, “it doesn’t work”? Couting a string by itself should always work; you should never need to use c_str() with it. Can you be more specific about what symptoms you’re actually seeing when you say things like “it doesn’t work”?

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 22:50

Sorry, you’re right I should be more specific.

When I call cout << “Received: “ << msg << “\n”; after receiving data, the output is simply:

Received:

It’s as if msg is empty. However replacing “msg” with “msg.c_str()” in the call gives an output closer to what the code should be doing.

I’ve attempted to chuck my recvAll() function back into my code with a call to resize() and I get the same problem as above. When I output the string it contains nothing.

RecvAll:

int RecvAll(TCPsocket sock, string msg)
{
    int total = 0;
    unsigned short len = 0;
    int bytesleft = 0;
    int r = 0;

    // Receive length
    r = SDLNet_TCP_Recv(sock, &len, sizeof(short));
    if (r < 0)
    {
        return -1;
    }
    else
    {
        bytesleft = (int)len;
        msg.resize(len);
    }

    // Receive message
    while (total < len)
    {
        r = SDLNet_TCP_Recv(sock, const_cast<char *>(msg.c_str()), min(512, bytesleft));
        if (r < 0)
        {
            return -1;
        }
        else
        {
            total += r;
            bytesleft -= r;
        }
    }

    return total;
}

And the call:

result = RecvAll(client[i], msg);
                
if (result < 0)
{
    cout << "Client " << i << " disconnected.\n";
    client[i] = NULL;
}
else
{
    cout << "Received: " << msg.c_str() << "\n";
}
A8433b04cb41dd57113740b779f61acb
0
Reedbeta 167 Aug 27, 2009 at 23:01

@King Tokio

When I call cout << “Received: “ << msg << “\n”; after receiving data, the output is simply:

Received:

It’s as if msg is empty. However replacing “msg” with “msg.c_str()” in the call gives an output closer to what the code should be doing.

Hmm, that’s bizarre. I don’t have an explanation for why it would be behaving that way. Maybe you could find out more by stepping through the program in the debugger and poking at the string innards.

17ba6d8b7ba3b6d82970a7bbba71a6de
0
vrnunes 102 Aug 27, 2009 at 23:18

Suggestion:

Client:
– Put a breakpoint at the send() line;
– Take a look at the buffer before it is sent. Is it correctly formatted?

Server:
– Put a breakpoint at the line after the recv() line;
– Take a look at the received buffer. Is it correctly formatted?

What do you see there, at both locations?

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 23:26

Well using the debugger I worked out that the problem with my recvAll() function is that I was passing msg by value so that at the end of the function call, msg hadn’t been affected (it was empty).

I still cannot figure out why (in the code I posted earlier) I cannot cout msg.

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 27, 2009 at 23:50

@vrnunes

Suggestion:

Client:
– Put a breakpoint at the send() line;
– Take a look at the buffer before it is sent. Is it correctly formatted?

Server:
– Put a breakpoint at the line after the recv() line;
– Take a look at the received buffer. Is it correctly formatted?

What do you see there, at both locations?

On the server side, after a call to:

result = RecvAll(client[i], msg);

result = 6 and msg = “”. The string sent was “Hello” so as you can see the data was received but not put in msg. I think this is because I have passed msg by value and it is only being changed inside the function call. Howver I do not know how to fix this.

On the client side, after a call to:

result = SDLNet_TCP_Send(tcpsock, msg.c_str(), min(BUFFER, (int)len));

result = 6, msg = “Hello”.

EDIT: Apologies for the double post.

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 167 Aug 27, 2009 at 23:54

You can pass msg by reference using the & symbol, e.g. “string & msg” instead of “string msg”, in the function parameter line.

Also, why are you getting len = 6, when “Hello” has only five characters?

4d01f236827d1303d28e38405c5f976d
0
King_Tokio 101 Aug 28, 2009 at 00:17

I was told to add 1 for the terminating null when I was using char* msg. I assumed it was the same with string?

And thanks Reedbeta, adding &msg in the function parameter line worked. I was incorrectly using &msg in the function call rather than the function declaration.

Now all I need is a way of getting user input without putting the rest of the program on hold and I’ll be a happy bunny (at least for a while).

EDIT: Doing the above also fixed the earlier problem I had with couting msg. I no longer have to use c_str() which is great. Still don’t know why that was happening.