changing to std::vector causing segmentation fault

Question

Hello so i have a struct that allocate memory using new keyword and i wanted to remove by using std::vector instead so i did like this

before changing to std::vector

BYTE* packPlayerMoving(PlayerMoving* dataStruct)
{
    BYTE* data = new BYTE[56];
    for (int i = 0; i < 56; i++)
    {
        data[i] = 0;
    }
    memcpy(data, &dataStruct->packetType, 4);
    memcpy(data + 4, &dataStruct->netID, 4);
    memcpy(data + 12, &dataStruct->characterState, 4);
    memcpy(data + 20, &dataStruct->plantingTree, 4);
    memcpy(data + 24, &dataStruct->x, 4);
    memcpy(data + 28, &dataStruct->y, 4);
    memcpy(data + 32, &dataStruct->XSpeed, 4);
    memcpy(data + 36, &dataStruct->YSpeed, 4);
    memcpy(data + 44, &dataStruct->punchX, 4);
    memcpy(data + 48, &dataStruct->punchY, 4);
    return data;
}

void SendPacketRaw(int a1, void *packetData, size_t packetDataSize, void *a4, ENetPeer* peer, int packetFlag)
{
    ENetPacket *p;

    if (peer) // check if we have it setup
    {
        if (a1 == 4 && *((BYTE *)packetData + 12) & 8)
        {
            p = enet_packet_create(0, packetDataSize + *((DWORD *)packetData + 13) + 5, packetFlag);
            int four = 4;
            memcpy(p->data, &four, 4);
            memcpy((char *)p->data + 4, packetData, packetDataSize);
            memcpy((char *)p->data + packetDataSize + 4, a4, *((DWORD *)packetData + 13));
            enet_peer_send(peer, 0, p);
        }
        else
        {
            p = enet_packet_create(0, packetDataSize + 5, packetFlag);
            memcpy(p->data, &a1, 4);
            memcpy((char *)p->data + 4, packetData, packetDataSize);
            enet_peer_send(peer, 0, p);
        }
    }
    delete (char*)packetData;
}

after changing to std::vector

std::vector<BYTE> packPlayerMoving(PlayerMoving *dataStruct)
{
    std::vector<BYTE> data(56);
    memcpy(&data[0], &dataStruct->packetType, 4);
    memcpy(&data[4], &dataStruct->netID, 4);
    memcpy(&data[12], &dataStruct->characterState, 4);
    memcpy(&data[20], &dataStruct->plantingTree, 4);
    memcpy(&data[24], &dataStruct->x, 4);
    memcpy(&data[28], &dataStruct->y, 4);
    memcpy(&data[32], &dataStruct->XSpeed, 4);
    memcpy(&data[36], &dataStruct->YSpeed, 4);
    memcpy(&data[44], &dataStruct->punchX, 4);
    memcpy(&data[48], &dataStruct->punchY, 4);
    return data;
}

void SendPacketRaw(int a1, std::vector<BYTE> packetData, size_t packetDataSize, void *a4, ENetPeer *peer, int packetFlag)
{
    ENetPacket *p;

    if (peer) // check if we have it setup
    {
        if (a1 == 4 && *(&packetData[12]) & 8)
        {
            p = enet_packet_create(0, packetDataSize + *(&packetData[13]) + 5, packetFlag);
            int four = 4;
            memcpy(p->data, &four, 4);
            memcpy((char *)p->data + 4, &packetData[0], packetDataSize);
            void * a4 = 0;
            memcpy((char *)p->data + packetDataSize + 4,  a4, *(&packetData[13]));
            enet_peer_send(peer, 0, p);
        }
        else
        {
            p = enet_packet_create(0, packetDataSize + 5, packetFlag);
            memcpy(p->data, &a1, 4);
            memcpy((char *)p->data + 4, &packetData[0], packetDataSize);
            enet_peer_send(peer, 0, p);
        }
    }
}

and i use in this way :

SendPacketRaw(4, packPlayerMoving(&data), 56, 0, currentPeer, ENET_PACKET_FLAG_RELIABLE);

ok so my problem is before i convert it to std::vector it was working fine without issues but after i converted it i started getting segmentation fault what am i doing wrong ?

and here valgrind output

==12418== Conditional jump or move depends on uninitialised value(s)
==12418==    at 0x1189D7: SendPacketRaw(int, std::vector<unsigned char, std::allocator<unsigned char> >, unsigned long, void*, _ENetPeer*, int) (utils.cpp:172)
==12418==    by 0x11E337: Nothing(_ENetPeer*, int, int) (worlds.cpp:405)
==12418==    by 0x1208AD: Events::Recieve(_ENetHost*, _ENetPacket*, _ENetPeer*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (events.cpp:315)
==12418==    by 0x123BDB: Run() (main.cpp:62)
==12418==    by 0x123C85: main (main.cpp:87)
==12418== 
==12418== Conditional jump or move depends on uninitialised value(s)
==12418==    at 0x1450E7: enet_packet_create (in /home/cmd/Desktop/PRC++/server)
==12418==    by 0x118A20: SendPacketRaw(int, std::vector<unsigned char, std::allocator<unsigned char> >, unsigned long, void*, _ENetPeer*, int) (utils.cpp:174)
==12418==    by 0x11E337: Nothing(_ENetPeer*, int, int) (worlds.cpp:405)
==12418==    by 0x1208AD: Events::Recieve(_ENetHost*, _ENetPacket*, _ENetPeer*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (events.cpp:315)
==12418==    by 0x123BDB: Run() (main.cpp:62)
==12418==    by 0x123C85: main (main.cpp:87)
==12418== 
==12418== Conditional jump or move depends on uninitialised value(s)
==12418==    at 0x48429FA: memmove (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==12418==    by 0x118AA8: SendPacketRaw(int, std::vector<unsigned char, std::allocator<unsigned char> >, unsigned long, void*, _ENetPeer*, int) (utils.cpp:179)
==12418==    by 0x11E337: Nothing(_ENetPeer*, int, int) (worlds.cpp:405)
==12418==    by 0x1208AD: Events::Recieve(_ENetHost*, _ENetPacket*, _ENetPeer*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (events.cpp:315)
==12418==    by 0x123BDB: Run() (main.cpp:62)
==12418==    by 0x123C85: main (main.cpp:87)
==12418== 
==12418== Conditional jump or move depends on uninitialised value(s)
==12418==    at 0x4842BA1: memmove (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==12418==    by 0x118AA8: SendPacketRaw(int, std::vector<unsigned char, std::allocator<unsigned char> >, unsigned long, void*, _ENetPeer*, int) (utils.cpp:179)
==12418==    by 0x11E337: Nothing(_ENetPeer*, int, int) (worlds.cpp:405)
==12418==    by 0x1208AD: Events::Recieve(_ENetHost*, _ENetPacket*, _ENetPeer*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (events.cpp:315)
==12418==    by 0x123BDB: Run() (main.cpp:62)
==12418==    by 0x123C85: main (main.cpp:87)
==12418== 
==12418== Conditional jump or move depends on uninitialised value(s)
==12418==    at 0x4842B0E: memmove (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==12418==    by 0x118AA8: SendPacketRaw(int, std::vector<unsigned char, std::allocator<unsigned char> >, unsigned long, void*, _ENetPeer*, int) (utils.cpp:179)
==12418==    by 0x11E337: Nothing(_ENetPeer*, int, int) (worlds.cpp:405)
==12418==    by 0x1208AD: Events::Recieve(_ENetHost*, _ENetPacket*, _ENetPeer*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (events.cpp:315)
==12418==    by 0x123BDB: Run() (main.cpp:62)
==12418==    by 0x123C85: main (main.cpp:87)
==12418== 
==12418== Invalid read of size 2
==12418==    at 0x4842B30: memmove (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==12418==    by 0x118AA8: SendPacketRaw(int, std::vector<unsigned char, std::allocator<unsigned char> >, unsigned long, void*, _ENetPeer*, int) (utils.cpp:179)
==12418==    by 0x11E337: Nothing(_ENetPeer*, int, int) (worlds.cpp:405)
==12418==    by 0x1208AD: Events::Recieve(_ENetHost*, _ENetPacket*, _ENetPeer*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (events.cpp:315)
==12418==    by 0x123BDB: Run() (main.cpp:62)
==12418==    by 0x123C85: main (main.cpp:87)
==12418==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==12418== 
==12418== 
==12418== Process terminating with default action of signal 11 (SIGSEGV)
==12418==  Access not within mapped region at address 0x0
==12418==    at 0x4842B30: memmove (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==12418==    by 0x118AA8: SendPacketRaw(int, std::vector<unsigned char, std::allocator<unsigned char> >, unsigned long, void*, _ENetPeer*, int) (utils.cpp:179)
==12418==    by 0x11E337: Nothing(_ENetPeer*, int, int) (worlds.cpp:405)
==12418==    by 0x1208AD: Events::Recieve(_ENetHost*, _ENetPacket*, _ENetPeer*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (events.cpp:315)
==12418==    by 0x123BDB: Run() (main.cpp:62)
==12418==    by 0x123C85: main (main.cpp:87)
==12418==  If you believe this happened as a result of a stack
==12418==  overflow in your program's main thread (unlikely but
==12418==  possible), you can try to increase the size of the
==12418==  main thread stack using the --main-stacksize= flag.
==12418==  The main thread stack size used in this run was 8388608.
==12418== 
==12418== HEAP SUMMARY:
==12418==     in use at exit: 11,557,918 bytes in 22,893 blocks
==12418==   total heap usage: 94,578 allocs, 71,685 frees, 29,567,151 bytes allocated
==12418== 
==12418== LEAK SUMMARY:
==12418==    definitely lost: 1,312 bytes in 4 blocks
==12418==    indirectly lost: 5,711 bytes in 17 blocks
==12418==      possibly lost: 224 bytes in 2 blocks
==12418==    still reachable: 11,550,671 bytes in 22,870 blocks
==12418==         suppressed: 0 bytes in 0 blocks
==12418== Rerun with --leak-check=full to see details of leaked memory
==12418== 
==12418== Use --track-origins=yes to see where uninitialised values come from
==12418== For lists of detected and suppressed errors, rerun with: -s
==12418== ERROR SUMMARY: 45563 errors from 34 contexts (suppressed: 0 from 0)
Segmentation fault (core dumped)

Answer 1

i think i see a major bug in your code, a std::vector<BYTE> data , in &data[0] if you do the address operator u get the object address as [] is a operator of vector class. so u may wanna use data.data() it'll return you a raw pointer, do this before memcpy()
ps: my first answer

Answer 2

*((DWORD *)packetData + 13) in your first version reads the 13th DWORD (at byte index 52).

*(&packetData[13]) in your second version reads the 13th BYTE.

This whole code is a bit of a mess, so I'm loath just to paper over that issue, but changing the second version to:

*(((DWORD *)packetData.data())+13) should fix it.

Answer 3

In your original code, inside your SendPacketRaw() function, you were delete-ing the packetData pointer parameter passed to it, while in this your modified version that second parameter packetData is a vector which continues to hold some data after the end of your function SendPacketRaw().

I'm not sure about the use of your SendPacketRaw() function and the logic of managing the memory that its second parameter is supposed to maintain, but I have a gut feeling that this is the main cause of Valgrind being unhappy.

So I advise you first of all to pass that second vector parmeter packetData as a reference, and to clean it before finishing your function.

void SendPacketRaw(int a1, std::vector<BYTE> &packetData, size_t packetDataSize, void *a4, ENetPeer *peer, int packetFlag)
{
    ENetPacket *p;

    if (peer) // check if we have it setup
    {
        if (a1 == 4 && *(&packetData[12]) & 8)
        {
            p = enet_packet_create(0, packetDataSize + *(&packetData[13]) + 5, packetFlag);
            int four = 4;
            memcpy(p->data, &four, 4);
            memcpy((char *)p->data + 4, &packetData[0], packetDataSize);
            void * a4 = 0;
            memcpy((char *)p->data + packetDataSize + 4,  a4, *(&packetData[13]));
            enet_peer_send(peer, 0, p);
        }
        else
        {
            p = enet_packet_create(0, packetDataSize + 5, packetFlag);
            memcpy(p->data, &a1, 4);
            memcpy((char *)p->data + 4, &packetData[0], packetDataSize);
            enet_peer_send(peer, 0, p);
        }
    }
    
    packetData.clear();
}