Packet handeling question

When using TCP you are not sending 'packets', you are sending a stream of data. Packets are whatever mechanism you use to delimit your data. For example, when you send '1' followed by the structure, that entire block of data could be read with a single recv() call on the server end. Sending the opcode followed by the struct is fine.

What you should be thinking about the most is what to do when you go to receive the payload and not all of the data has arrived yet. It is very possible that if your payload is large enough that the transport layer fragments your data, so one single recv() will not get the entire contents. You will need a receive buffering scheme so that you can get some of the data now, and the rest when it arrives later.

The mechanism I've been using for quite a while now is I prepend all of my chunks of data with a header. The header contains a sync byte ( for sanity ) the op code, and the size of the payload. The recv loop doesn't know anything about the contents of the payload, but because of the header it can still receive the data. When it has received the entire payload, it calls a handler function that knows how to handle the payload. ( In my case the handler is a big switch statement that calls opcode specific handler functions )

--Z

if (opcode == "1") {[/quote]

The specific problem here is that you're comparing pointer values, which will never be equal. You want to test if (opcode[0] == '1'). And you don't want to use strcmp(), because it's not guaranteed that the data sent by the client is zero terminated (in fact, in this case, it will never be).

The other problem is that the second recv() is not guaranteed to return data for the entire packet. You can get around that by wrapping the code in something like a "recv_all()" or just calling read() instead (if you're on UNIX). However, that would mean that an attacker could send the byte '1' to you, and then send nothing more, and your program would just sit inside recv_all() waiting forever.

You really want to write your data reading code as a state machine with a buffer. Something like:

if (socket is readable from select) {  recv(socket, buffer-write-position, space-remaining-in-buffer);  if (buffer is not empty) {    while (buffer-data-size >= data-needed-for-packet-of-type(buffer[0])) {      handle-packet-at-front-of-buffer      slide-buffer-over-so-remaining-data-is-first    }  }}

I appreciate the responses, and will try again with the advice in mind.
So are you saying there should just be a main recv buffer that reads all the data and then processes it later, and not just a opcode/packet being sent 1 after another, and read right after (Having the server expect the packet right after the opcode) ? I seemed to have a problem where the server got the opcode (That part works fine) but the packet 'seemed' to be read by the server too soon thus nothing being in the recv buffer. My ideal goal was to just send off 1 packet (struct) and recv 1 packet (struct) (With header/opcode/packet data -- possible the same thing Zimans was talking about -- ) but the server (and client for that matter) has no way of knowing which struct to call recv() on.

Also as a side note, the above code is not necessarily the code I am using. It's just to show you the concept of what I am doing and what I am trying to do.

Reading in an opcode, then a struct is fine, you just have to handle the case where a full packet hasn't arrived yet. With a small packet this is less likely to happen, but when you start sending larger pieces of data you need to be aware that the entire chunk of data may not have been received yet, as the underlying transport layer may break up the packets.

Don't get into poor habits. When writing single-user and/or back-end programs, similar to the UNIX fork() model, it's OK to carry on a TCP conversation where you block until you get the data you want. For example, if you have an SMTP server (or client) and fork() for each incoming client, that's not a problem.

With games, life is different. There are a number of players that all need to be served at the same time, and that all share the same state. To make it worse, the state updates and needs to be coherent within milliseconds -- the SQL server model isn't very good for games, either. Thus, in these cases, you really should split the work into two separate parts:

1) Drain all incoming sockets into buffers per-connection.
2) Process fully available packets in the buffers per-connection.

You may be able to split those into separate threads, if you want -- in that case, make the "check for packet and remove from queue" operation quick and atomic, copying the data out, rather than blocking socket draining on packet processing.

In pseudo-code:

  select(all connected sockets);  foreach (connection with a readable socket)    read data into buffer for connection, if buffer not already full  foreach (connection)    foreach (fully available message in buffer)      remove and process message

I see. Thanks for both of your help, I will revise my code and see how it works. Will post back on this thread if I need further assistance. Keep on coding!