Segmentation Fault during delete []

Question

I am building a game and I need to store in a dynamic array a player, every time that a player is created. I built a small piece of code to try it and I get a segmentation fault when I try to delete the table to insert the third player. I just can't realize why that happens: the header file is:

    #include <iostream>
    #include <string>
    #include <vector>
    
    using namespace std;
    
    class Player

{

    private:
        string PlayerName;
        int PlayerScore;
    public:
        Player();       
        Player(string name, int s);     
        ~Player() {};       
        string getPlayerName()  {return PlayerName;}
        int getPlayerScore() {return PlayerScore;}
        void setPlayerName(string name){PlayerName = name;}
        void setPlayerScore(int score){PlayerScore = score;}
};

class Game
{

    private:
        Player NewPlayer;
        int NPlayers;
        Player* PlayerList;
        //vector <Player> PlayerList2;
    
        
    public:
        Game();
        ~Game();
        void setNewPlayer(string, int); 
        void resizePlayerList();
        void PrintList();   
};

The class file:

#include <iostream>
#include <string>
#include <cstring>
#include <memory>
#include "I.h"

using namespace std;


    
Player::Player()
{
    PlayerName = "";
    PlayerScore = 0;
}

Player::Player(string name, int s)
{
    PlayerName = name;
    PlayerScore = s;
}       


Game::Game() 
{
    NPlayers = 0;
    PlayerList = NULL;
};
Game::~Game() {};

void Game::setNewPlayer(string str, int scr)
{
    NewPlayer = Player(str, scr);   
    resizePlayerList(); 
    PrintList();        
}


void Game::resizePlayerList() {
   
    if(NewPlayer.getPlayerName() != "No Name")
    {
            int newSize = NPlayers +1;
            
            Player* newArr = NULL;
            newArr = new Player[newSize];           
            memcpy( newArr, PlayerList, NPlayers * sizeof(Player) );            
            NPlayers = newSize;         
            delete [] PlayerList;           
            PlayerList = newArr;            
            PlayerList[NPlayers-1] = NewPlayer;
    } 
   
}


void Game::PrintList()
{
    Player player;
    //cout << NPlayers << endl;
    for(int i= 0; i < NPlayers; i++)
    {
        player = PlayerList[i];     
        cout << player.getPlayerName() << " " <<  player.getPlayerScore() << endl;  
    }

}

The main:

#include <iostream>
#include <string>
#include "I.h"

using namespace std;

int main()
{
    Game NewGame;
    NewGame.setNewPlayer("Peter",20);
    NewGame.setNewPlayer("Someone Someone",30);
    NewGame.setNewPlayer("Someone else",40);
    return 0;
}

In modern C++, you should use containers and smart pointers, rather than try to manage that yourself (because it's hard, and you'll make bugs... like you did here... using `memcpy` on an object that cannot be `memcpy`'d). — Eljay, Apr 09 '22 at 14:16
"I need to store in a dynamic array a player" -> `std::vector` — 463035818_is_not_an_ai, Apr 09 '22 at 14:21
if you do manually manage memory you need to read this: https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three, but you really shouldn't do it manually — 463035818_is_not_an_ai, Apr 09 '22 at 14:22
`std::vector playerList;` then resizing is as trivial as `playerList.push_back(NewPlayer);` — AndersK, Apr 09 '22 at 14:27

ChrisMM · Accepted Answer · 2022-04-09T14:31:35.540

5

The problem stems from here:

memcpy(newArr, PlayerList, NPlayers * sizeof(Player));

You cannot copy classes in this manner, unless they are Trivially Copyable (originally said POD, but as Yksisarvinen points out in the comments, memcpy is not quite that restrictive). You can fix this by using a loop to copy over the data instead:

for (int i = 0; i < NPlayers; ++i) {
    newArr[i] = std::move(PlayerList[i]);
}

The better option though, is to use a std::vector<Player>, then the entire resize function can be simplified to:

void Game::resizePlayerList() {
    if (NewPlayer.getPlayerName() != "No Name") {
        PlayerList.push_back(std::move(NewPlayer));
    }
}

edited Apr 09 '22 at 14:31

answered Apr 09 '22 at 14:24

ChrisMM

8,448
13
29
48

Or the `` function `std::move` instead of the loop. – aschepler Apr 09 '22 at 14:30
@aschepler, honestly didn't even know that was thing. That's good to know for future reference :) – ChrisMM Apr 09 '22 at 14:33

Segmentation Fault during delete []

1 Answers1