-1

I'm attempting to interact with a 2D array thorough the Board class. However, I'm getting a segmentation fault when running the main file containing this code:

#include "Board.h"

int main(int argc, char** argv)
{
  int height = 0;
  int width = 0;
  int pop_density = 0.8;

  Board* c = new Board(height,width);
  c->print();
  c->populate(pop_density);
  c->print();

  //for (i )
  cout << c->read_char_at_index(1,2) << endl;

  delete c;

  return 0;
}

This is the Board.cpp Code:

#include "Board.h"

//in board: make a fucntion that pulls from file

Board::Board(int h, int w)
{
  m_height = h;
  m_width = w;
  m_array = new char* [m_height];
  for (int i = 0; i < m_height; ++i)
  {
    m_array[i] = new char[m_width];
    for (int j = 0; j < m_width; ++j)
    {
      m_array[i][j] = '-';
    }
  }
  cout << "Made board" << endl;
}

Board::~Board()
{
  for (int i = 0; i < this->m_height; ++i)
  {
    delete[] this->m_array[i];
  }
  delete[] this->m_array;

  cout << "Destructed Board" << endl;
}

void Board::print()
{
  for (int i = 0; i < this->m_height; ++i)
  {
    for (int j = 0; j < this->m_width; ++j)
    {
      cout << this->m_array[i][j] << " ";
    }
    cout << endl;
  }
}

void Board:: populate(double density)
{
  //seeding rand with time
  srand(time(NULL));
  int totalCells = this->m_height * this->m_width;
  int cellsToFill = round(totalCells * density);
  int cellsFilled = 0;

  for (int i = 0; i < cellsToFill; ++i)
  {
    int randomRow = rand() % this->m_height;
    int randomColumn = rand() % this->m_width;

    this->m_array[randomRow][randomColumn] = 'X';
  }
}

void Board:: write_char_at_index(int height, int width, char z)
{
  cout << "pre" << endl;
  cout << height << " " << width << endl;
  m_array[height][width] = z;
  cout << "Wrote" << endl;
}

char Board:: read_char_at_index(int height, int width)
{
  return m_array[height][width];
  cout << "read" << endl;
}

And the Board.h Code:

#ifndef BOARD_H
#define BOARD_H

#include <iostream>
#include <string>
#include <cmath>
#include <cstdlib>
#include <ctime>
using namespace std;

//This class is used to make a Board object

class Board
{
  public:
    Board(int h, int w);
    ~Board();
    void print(); // Prints the board to cout
    void populate(double density); // Populates board based on density input
    void write_char_at_index(int height, int width, char z);
    char read_char_at_index(int height, int width);

  private:
    char** m_array; // 2D array dynamically allocated during runtime
    int m_height;
    int m_width;
};

#endif

Any help or advice would be great, as I've already asked two classmates and they're not sure what the problem is. I know for sure the index I'm trying to assign the char value to is not out of bounds.

Ðаn
  • 10,934
  • 11
  • 59
  • 95
yupthatsme
  • 47
  • 7
  • What are the values you're passing to function `write_char_at_index`? Is it possible they're out of bounds? – Reticulated Spline Mar 06 '20 at 02:11
  • 1
    you might be copying the object, explicitly or inadvertently. if so, the default constructor will just copy the m_array pointer, then the original object gets destroyed and frees the memory – IMil Mar 06 '20 at 02:13
  • most likely, if you have a function "DoSomethingWith(Board board)" instead of "DoSomethingWith(const Board & board)", the board gets destroyed on exit from the function – IMil Mar 06 '20 at 02:16
  • I've checked the values I'm passing and none of them are out of bounds, the function is also inside the Board class, so it's never being passed a board, rather referencing the object's own internal array. – yupthatsme Mar 06 '20 at 02:21
  • Not much to go on here. I'd follow @IMil 's advice and make sure you have the [Rule of Three (and friends)](https://en.cppreference.com/w/cpp/language/rule_of_three) covered. – user4581301 Mar 06 '20 at 02:28
  • 1
    Unrelated: [A much easier (and usually faster) way to do a 2D array](https://stackoverflow.com/a/2076668/4581301). – user4581301 Mar 06 '20 at 02:29
  • 1
    Consider producing a [mcve] and if that doesn't point out the error and its solution, edit your post and replace the code with the MRE. – user4581301 Mar 06 '20 at 02:31
  • @yupthatsme you seem to misunderstand. While this particular method is "inside" the class, you are calling it somehow, and that's where the problem may be. e.g. if you do something like " Board x(...); f1(x); x.print_char_at(...); ", and f1 takes Board by value, then it frees memory on exit, and the next call crashes – IMil Mar 06 '20 at 02:44
  • 1
    Since you seem to be relying on screen messages to debug, I'd recommend to insert a log message "The memory just got freed" in the destructor. Quite likely you'll see it before the output from your method. – IMil Mar 06 '20 at 02:48
  • Try compiling using address sanitizer if you have access to gcc or clang. If you're on linux, you can use valgrind as well. – Stephen Newell Mar 06 '20 at 03:33
  • BTW, you can reduce your typing by removing the `this->` syntax. The syntax is only needed to differentiate member names from parameter names (change one and the syntax isn't necessary). – Thomas Matthews Mar 06 '20 at 04:50

1 Answers1

1

The input parameters of read_char_at_index are (1,2) in the main function. However, The Board object c is with height = 0 and width = 0. You can modify the value of height and width to large enough values, such as 10, 10.

    int height = 10;
    int width = 10;
    int pop_density = 0.8;

    Board* c = new Board(height, width);
    c->print();
    c->populate(pop_density);
    c->print();

    cout << c->read_char_at_index(1, 2) << endl;

The program may run without Segmentation Fault.

Note: The functions "write_char_at_index" and "read_char_at_index" may check the input values (height and width) to ensure the access to m_array member is safe.

    if (height >= this->m_height || width >= this->m_width)
    {
        return;
    }
JimmyHu
  • 403
  • 1
  • 8
  • 20
  • 2
    Side note: `Board* c = new Board(height, width);` could also be `Board c(height, width);` There is no need for dynamic allocation here. – user4581301 Mar 06 '20 at 16:38
  • Thank you for your comments. Moreover, the object c wouldn't be deleted manually in the case of using `Board c(height, width);`. – JimmyHu Mar 06 '20 at 18:16