CODEWITHSUNDEEP
c++segmentation-fault
Austin Moore
Austin Moore

Reputation: 1412

Segmentation fault: 11 - Where is it?

I am compiling a program with gcc and when I run a.out I get this error:

Segmentation fault: 11

I think it has to do with Board::display, but I don't see anything wrong with it..

#include <iostream>

using namespace std;

const bool WHITE = 0;
const bool BLACK = 1;

//-----------------------------

class Piece
{
public:
    // constructors
    Piece();
    Piece(bool color);

    // functions
    char getColor()  const {return color; }
    bool getSymbol() const {return symbol;}

protected:
    bool color;
    char symbol;
};

ostream& operator << (ostream& out, const Piece & piece)
{
    out << piece.getSymbol();
    return out;
}

Piece::Piece() { }

Piece::Piece(bool color)
{
    this->color = color;
}

//-----------------------------

class King : public Piece
{
public:
    King(bool color);
};

King::King(bool color) : Piece(color)
{
    this->symbol = 'K';
}

//-----------------------------

class Tile
{
public:
    // constructors/destructors
    Tile();
    Tile(Piece piece, int row, int col);

    // functions
    bool  getColor()  const {return color;}
    void display();

private:
    Piece piece;
    int row, col;
    bool color;

};

Tile::Tile() { }

Tile::Tile(Piece piece, int row, int col)
{
    this->row = row;
    this->col = col;
}

void Tile::display()
{
    if (getColor() == BLACK)
        {
            if (piece.getColor() == BLACK)
                cout << "\E[30;47m " << piece << " \E[0m";
            else
                cout << "\E[37;47m " << piece << " \E[0m";
        }
        else
        {
            if (piece.getColor() == BLACK)
                cout << "\E[30;41m " << piece << " \E[0m";
            else
                cout << "\E[37;41m " << piece << " \E[0m";
        }
}

//---------------------------

class Board
{
public:
    // constructor
    Board();

    // functions
    void display();

private:
    Tile *tiles[8][8];
};

Board::Board()
{
    for (int r = 0; r < 8; r++)
        for(int c = 8; c < 8; c++)
            tiles[r][c] = new Tile(King(BLACK), r, c);
}

void Board::display()
{
    for (int r = 7; r >= 0; r--)
    {
        for(int c = 7; c >= 0; c--)
            tiles[r][c]->display();
        cout << endl;
    }
}

//---------------------------

int main() 
{
    Board board;

    board.display();
}

Upvotes: 1

Views: 1870

Answers (4)

uesp
uesp

Reputation: 6204

Unless it is just a typo the line:

for(int c = 8; c < 8; c++)

in the Board constructor is wrong and will result in none of tiles being allocated with the ultimate result of "bad" things happening when you try to use them.

Change it to:

for(int c = 0; c < 8; c++)

At some point you'll want to delete these although a better option might to be not use pointers at all in this case, just Tile tiles[8][8]; or a vector<vector<Tile> > tiles.

Upvotes: 0

Kerrek SB
Kerrek SB

Reputation: 477060

In Board::display(), r++ should be r--, and ditto for c++.

If (like me) you prefer unsigned variables for array indices, you would write the loop like this:

for (std::size_t i = 0; i != N; ++i)
{
    array[N - 1 - i] = something();
}

Or, if you find that to cumbersome but you still really don't like <= and prefer iterator-style "not-equals" termination (like me), you can stick with unsigned types and say:

for (std::size_t i = N; i != 0; --i)
{
    array[i - 1] = anotherthing();
}

Your next mistake is to store a polymorphic object by value of the base. That can't work! Instead, you might want to save a pointer (or reference). While we're at it, it's time for you to learn constructor initializer lists:

class Tile
{
    Piece * piece;    // must be a polymorphic handle!
    int row;
    int col;

public:
    Tile(Piece * p, int r, int c) : piece(p), row(r), col(c) { }
    Tile() : piece(NULL), row(0), col(0) { }
};

If you store a polymorphic object by value, you end up slicing it. You could have saved yourself the trouble by making Piece abstract.

As you can see, you should also make sure the default constructor does something useful. In fact, I would argue that the default constructor doesn't actually make sense, and that you should just remove it.

Finally, I should add that the Tile class design is a terrible problem, because you don't manage the lifetime of the Piece objects, which currently just leak. If you were to manage them, you'd need to delete the piece in the destructor of Tile, but then you'd need to implement the Rule of Three, and now you realise that Piece needs a virtual destructor...

I'm realising that this is turning into a full-blown chapter of "How to do C++", so I'll stop now. I think we have several good books on our recommended list; please consult those.

Upvotes: 6

Luchian Grigore
Luchian Grigore

Reputation: 258618

Surely you mean:

for (int r = 7; r >= 0; r--)
{
    for(int c = 7; c >= 0; c--)

to be

for (int r = 6; r > 0; r--)
{
    for(int c = 6; c > 0; c--)

Upvotes: 0

Marlon
Marlon

Reputation: 20312

In Board::display, you are incrementing your variables in the loops instead of decrementing them.

You also aren't assigning piece to Tile::piece in Tiles constructor. (this->piece = piece)

Upvotes: 3

Related Questions