CODEWITHSUNDEEP
c++memcpy
Dries
Dries

Reputation: 1005

crash after memcpy: access violation reading location

if (m_Connections[t].socket != INVALID_SOCKET)
{
    m_TCPResult = recv(m_Connections[t].socket, m_TCPRecvbuf, m_TCPRecvbuflen, 0);

    if (m_TCPResult > 0)
    {
        printf("[TCPReceive] Bytes (m_TCPResult) received from %d: %d\n", m_Connections[t].socket, m_TCPResult);

        // Deserialize the data
        Packets::MainPacket receivedData;
        memcpy(&receivedData, m_TCPRecvbuf, sizeof(receivedData));

        // Check the type and do something with the data
        CheckType(m_Connections[t].socket, receivedData);
    }
    else  
    {
        if (WSAGetLastError() != WSAEWOULDBLOCK)
            printf("TCPReceive error: %d\n", WSAGetLastError());
    }
}

So I have this piece of code. I need to do a memcpy() to convert the incoming data from winsock to a struct that can be read by the application. However, after the CheckType() method is done the application crashes giving me an Access Violation Reading Location error. I removed the memcpy() method once to check and then it worked fine (no crashes).

I have no idea what the problem might be. I've been searching on Google but haven't found anything useful that seems to be a solution to my problem

EDIT:

Some more info:

// in the header
char m_TCPRecvbuf[DEFAULT_BUFLEN];

// receivedData struct
struct MainPacket
{
    char type;
    int id;

    LoginData loginData;
    vector<PlayerData> playerData;
};

Upvotes: 0

Views: 1554

Answers (1)

Sean
Sean

Reputation: 62502

You're writing over the vector when you do your memcpy. It's not a POD you can't initialize it via a memcpy, but instead have to use it's member functions to initialize it.

Think of it this way, the vector will have a pointer to the data it manages and a size_t indicating the size, at a minimum. You can't just initialize the pointer by memcpying a value you've received over the network. The pointer may make sense to the sender, but when you receive it all you've got is a pointer that's valid on the server, not in your application. Because of this the moment you try to use the vector you'll get undefined behaviour and will probably crash (if you're lucky).

Also, as a result of this sizeof doesn't work in the way you'd expect when applied to classes. For example, if you've got vector with 1,000 items in it then sizeof won't reflect this. What sizeof tells you is the combined size of all the member variables in the class definition (subject to padding). If our vector implementaton is just a pointer and a size_t then it'll probably be around 8 bytes on a 32bit platform, and 16 on a 64bit platform, regardless of how many items are in the vector.

What you need to do is encode information in the packet so that you can decode it. For example, rather than send a vector your packet should contain a field indicating the number of PlayerData instances, followed by the data for each player.

Upvotes: 1

Related Questions