Corruption of data in memcpy

Question

I'm currently working on a project using sockets via WinSock and have come across a peculiar problem. I'll attach the code before I start explaining.

#include "Connection.h"


Connection::Connection(SOCKET sock, int socketType)
    : m_sock(sock), m_recvCount(0), m_sendCount(0), m_socketType(socketType)
{
    printf("Succesfully created connection\n");
}


Connection::~Connection(void)
{
    printf("Closing socket %d", m_sock);
    closesocket(m_sock);
}

void Connection::ProcessMessage(const NetMessage *message){
    printf("Got network message: type %d, data %s\n", message->type, message->data);
}


bool Connection::ReadSocket(){
    // Call this when the socket is ready to read.
    // Returns true if the socket should be closed.

    // used to store count between the sockets
    int count = 0;

    if(m_socketType == SOCK_STREAM){
        // attempt to read a TCP socket message
        // Receive as much data from the client as will fit in the buffer.
        count = recv(m_sock, &m_recvBuf[m_recvCount], sizeof(m_recvBuf) - m_recvCount, 0);
    }
    else if(m_socketType == SOCK_DGRAM){
        // attempt to read UDP socket message

        // temporarily stores details of the address which sent the message
        // since UDP doesn't worry about whether it's connected to the
        // sender or not

        sockaddr_in fromAddr;
        int fromAddrSize = sizeof(fromAddr);

        count = recvfrom(m_sock, &m_recvBuf[m_recvCount], sizeof(m_recvBuf) - m_recvCount, 0, (sockaddr*) &fromAddr, &fromAddrSize);
    }
    else{
        printf("Unknown socket type %d\n", m_socketType);
        return true;
    }


    if (count <= 0)
    {
        printf("Tried to receive on socket %d and got %d bytes\n", m_sock, count);
        printf("Client connection closed or broken\n");
        return true;
    }

    // if we get to this point we have essentially received a complete message
    // and must process it

    printf("Received %d bytes from the client (total %d)\n", count, m_recvCount);
    m_recvCount += count;

    // Have we received a complete message?
    // if so, process it
    if (m_recvCount == sizeof NetMessage)
    {
        ProcessMessage((const NetMessage *) m_recvBuf);
        m_recvCount = 0;
    }

    return false;
}

bool Connection::WriteSocket(){
    // Sends the data in the send buffer through the socket

    int count;

    if(m_socketType == SOCK_STREAM){
        // attempt to read TCP socket message
        count = send(m_sock, m_sendBuf, m_sendCount, 0);
    }
    else if(m_socketType == SOCK_DGRAM){
        // attempt to read UDP socket message
        count = sendto(m_sock, m_sendBuf, m_sendCount, 0, 0, 0);
    }
    else{
        // unhandled type of socket, kill server
        printf("Unknown socket type %d", m_socketType);
        return true;
    }

    if (count <= 0)
    {
        // we have received an error from the socket
        printf("Client connection closed or broken\n");
        return true;
    }

    m_sendCount -= count;
    printf("Sent %d bytes to the client (%d left)\n", count, m_sendCount);
    printf("Data: %s", m_sendBuf);

    // Remove the sent data from the start of the buffer.
    memmove(m_sendBuf, &m_sendBuf[count], m_sendCount);

    return false;
}

bool Connection::WantWrite(){
    if(m_sendCount > 0){
        return true;
    }

    return false;
}

bool Connection::WantRead(){
    return true;
}

bool Connection::SetMessage(const NetMessage *message){
    // store contents of the message in the send buffer
    // to allow us to send later
    if (m_sendCount + sizeof(NetMessage) > sizeof(m_sendBuf))
    {
        return true;
    }

    memcpy(&m_sendBuf, message, sizeof(message));
    m_sendCount += sizeof(NetMessage);

    return false;
}

and the protocol

/* Definitions for the network protocol that the client and server use to communicate */

#ifndef PROTOCOL_H
#define PROTOCOL_H

// Message types.
enum MessageType
{
    MT_UNKNOWN = 0,
    MT_WELCOME = 1,
    MT_KEYPRESS = 2,
    MT_CHATMESSAGE = 3
};

// The message structure.
// This is a "plain old data" type, so we can send it over the network.
// (In a real program, we would want this structure to be packed.)
struct NetMessage
{
    MessageType type;
    char* data;

    NetMessage()
        : type(MT_UNKNOWN)
    {
    }
};

#endif

Essentially the protocol holds the definition of the messages that the client and server throw around to each other. The problem I am having is that, in connection.cpp line 132 (memcpy), the message becomes garbled in sendBuf.

http://imgur.com/MekQfgm,9ShRtHi

The image above shows exactly what is happening. As said in protocol.h the struct is a POD so when I do memcpy it should transfer the number of bytes as is held in the struct (so for example the message type should be 1 byte, followed by 7 or 8 bytes of data, in the example).

Can anyone shed some light on this? It's driving me crazy.

Answer 1

The line you wrote will copy 4 bytes (sizeof(pointer)) on 32bit systems:

memcpy(&m_sendBuf, message, sizeof(message));

what you probably meant is:

memcpy(&m_sendBuf, message, sizeof(NetMessage));

Edit:

In addition, as a commenter remarked, your data type is NOT a POD. It holds a pointer. You transfer that pointer. At the target system, it will point to the same place in RAM, but there will not be anything there. You need to actually make your datatype a POD by using an array or you need to find a way to transfer the data pointed to. You can achieve this by transfering the type, a length and a number of characters. That means that your receiver can NOT rely on messages being of fixed size.