Running into a heap corruption with WinSock2 application

Question

I have a feeling it's a stupid error, but I am making a echo server to identify packets, and output the data, then resend it back. It works on some packets, but one I'm trying to echo is breaking and saying there is a heap being corrupted.

Vector class, just using as container for now:

class vec2
{
public:
     vec2(){};
     float x, y;
};

PlayerData struct:

struct PlayerData
{
    vec2 pos;
    int health;
    float rotation;
    char moveflags;
    short playerID;
};

Packet I'm trying to send:

struct ServerPacket_SyncGame //5
{
     short packetID;
     PlayerData data[8];
};

The next part is messy, but I'll comment it to try to make sense.

ServerPacket_SyncGame* SP_SG = new ServerPacket_SyncGame; //creates packet pointer
for(int i = 0; i < 8; i++) //assigns the eight playerdata structs in the packet array
{
    SP_SG->data[i].playerID = i;
    SP_SG->data[i].health = rand() % 30;
    SP_SG->data[i].moveflags = 'D';
    SP_SG->data[i].pos.x = rand() % 1000;
    SP_SG->data[i].pos.y = rand() % 1000;
    SP_SG->data[i].rotation = rand() % 360;
}
SP_SG->packetID = 5; //assigns the packet id

cout << "\n\nSent data: \n"; ////Outputting the data to be sent
for(int i = 0; i < 8; i++)
    cout << "\nPlayer ID: " << SP_SG->data[i].playerID << "\nPosition: (" 
         << SP_SG->data[i].pos.x << ", " << SP_SG->data[i].pos.y 
         << ")\nHealth: " << SP_SG->data[i].health << "\nRotation: " 
         <<SP_SG->data[i].rotation << "\nMove Flags: "
         << SP_SG->data[i].moveflags << endl;

void* SP_SG_DAT = (void*)SP_SG; //casting the packet into a void*

char* SP_SG_BUFF = (char*)SP_SG_DAT; //casting from a void* to a char*

send(Socket, SP_SG_BUFF, sizeof(ServerPacket_SyncGame), 0); //sends the char*

char* SP_SG_RCVBUFF = new char; //new buffer for recv

recv(Socket, SP_SG_RCVBUFF, sizeof(ServerPacket_SyncGame), 0); //recv new buffer

void* SP_SG_RCVDAT = (void*) SP_SG_RCVBUFF; //casts char* to void* again

ServerPacket_SyncGame* RCVSP_SG = (ServerPacket_SyncGame*) SP_SG_RCVDAT;
//casts from void* to packet*

cout << "\n\nRecieved Data:\n\n";  //outputs converted received information
for(int i = 0; i < 8; i++)
    cout << "\nPlayer ID: " << SP_SG->data[i].playerID << "\nPosition: (" 
         << SP_SG->data[i].pos.x << ", " << SP_SG->data[i].pos.y 
         << ")\nHealth: " << SP_SG->data[i].health << "\nRotation: " 
         <<SP_SG->data[i].rotation << "\nMove Flags: "
         << SP_SG->data[i].moveflags << endl;

I've used this method with other packets, and it's worked perfectly, server side this is how it echos:

for(;;)
     {
          char* buffer = new char;
          char* temp = new char;
          int size = recv(Socket, buffer, sizeof(ServerPacket_SyncGame), 0);
          memcpy(temp, buffer, size);
          send(Socket, (char*)InterpretInfo((void*)temp), size, 0);
     };

InterpretInfo accepts the void* that you cast out of the char* you recieve, it deals with it like this:

void* InterpretInfo(void* data)
{

     short* tempsht = static_cast<short*>(data);
     cout << "\n\nRecieved packet ID: " << *tempsht;

     switch(*tempsht)
     {

This particular packet's ID is 5, this is it's case:

case 5:
         //ServerPacket_SyncGame
         {
           cout << " which is ServerPacket_SyncGame\n";
           ServerPacket_SyncGame* decoded = (ServerPacket_SyncGame*)data;
           for(int i = 0; i < 8; i++)
           {
            cout << "Player ID: " << decoded->data[i].playerID ;
            cout << "\nPosition: (" << decoded->data[i].pos.x << ", " 
                             << decoded->data[i].pos.y << ")\n";
            cout << "Health: " << decoded->data[i].health 
                             << "\nRotation: " << decoded->data[i].rotation 
                 << "\nMove Flags: " << decoded->data[i].moveflags << endl;
           }
            return(void*)decoded;
    }

It only does it with this packet, and it breaks and says the heap is corrupted when I try to access anything from the packet, though in debug mode I can read all information in the packet clearly.

I need at least 10 rep to post a picture, so here is a link to what I'm talking about in the running code: http://i.imgur.com/Dbyi0c3.png

Thank you in advanced for any help or insight to help me get this done, I'm still quite novice at C++ and love to learn.

Answer 1

You are allocating only 1 byte for the recv() buffer, but you are trying to read sizeof(ServerPacket_SyncGame) number of bytes into it. You need to change this:

char* SP_SG_RCVBUFF = new char; //new buffer for recv

To this:

char* SP_SG_RCVBUFF = new char[sizeof(ServerPacket_SyncGame)];

Same thing with your for loop:

for(;;)
{
    //char* buffer = new char;
    char* buffer = new char[sizeof(ServerPacket_SyncGame)];
    //char* temp = new char;
    char* temp = new char[sizeof(ServerPacket_SyncGame)];
    ...
 };

I would suggest you clean up your code:

ServerPacket_SyncGame* SP_SG = new ServerPacket_SyncGame; //creates packet pointer
for(int i = 0; i < 8; i++) //assigns the eight playerdata structs in the packet array
{
    SP_SG->data[i].playerID = i;
    SP_SG->data[i].health = rand() % 30;
    SP_SG->data[i].moveflags = 'D';
    SP_SG->data[i].pos.x = rand() % 1000;
    SP_SG->data[i].pos.y = rand() % 1000;
    SP_SG->data[i].rotation = rand() % 360;
}
SP_SG->packetID = 5; //assigns the packet id
...
// don't forget to do error handling on this, and pay attention to the
// return value so you know if you actually sent the entire struct or not...
send(Socket, (char*)SP_SG, sizeof(ServerPacket_SyncGame), 0);
delete SP_SG;

SP_SG = new ServerPacket_SyncGame;

// don't forget to do error handling on this, and pay attention to the
// return value so you know if you actually received the entire struct or not...
recv(Socket, (char*)SP_SG, sizeof(ServerPacket_SyncGame), 0); //recv new buffer
...
delete SP_SG;

ServerPacket_SyncGame buffer;
for(;;)
{
    // don't forget to do error handling on this, and pay attention to the
    // return value so you know if you actually received the entire struct or not...
    int size = recv(Socket, &buffer, sizeof(ServerPacket_SyncGame), 0);
    if (size > 0)
    {
        // don't forget to do error handling on this, and pay attention to the
        // return value so you know if you actually sent the entire struct or not...
        send(Socket, (char*)InterpretInfo(&buffer), size, 0);
    }
 };