CODEWITHSUNDEEP
c++multidimensional-array
sanitizedUser
sanitizedUser

Reputation: 2115

How to explain Segmentation fault sometimes happening when dynamically allocating 2D array?

I want to have a function that initializes simple Grid (2d array) with Columns and Rows, each position (cell) is then the size of struct.

I found a solution that accomplishes this in main function but when done in any other function, after running half-way through it crashes with segmentation fault before it prints the Grid (unlike in the last paragraph).

However, if printing the Grid is added directly behind the initializing part, the code works normally afterwards and all faults disappear.

I suspect that main does not now that Position array was initialized but I am passing it as pointer so, what am I doing wrong?

The following code is divided to two parts. First has the segmentation fault, second does not. The only difference is that in the second part, the for-cycles for printing out the grid are inside function that initializes the 2d array.

//SEGMENTATION FAULT

void CreateMap (struct GameGrid *Position, int &Dim_x, int &Dim_y)
{
    cout << "Lets create the game map." << endl;
    cout << "Enter number of Columns: ";
    cin >> Dim_x;
    cout << "Enter number of Rows: ";
    cin >> Dim_y;

    Position = new GameGrid[Dim_x * Dim_y];
}

int main()
{
    struct GameGrid *Position = NULL;
    int Dim_x;
    int Dim_y;

    CreateMap(Position, Dim_x, Dim_y);

    for (int y=0; y < Dim_y; y++)
    {
        cout << setw (20);

        for (int x=0; x < Dim_x; x++)
        {
            cout << Position[x*Dim_y + y].Element;
            cout << char(32);
        }
        cout << endl;
    }

    delete[] Position;
    return 0;
}

//NO FAULTS

void CreateMap (struct GameGrid *Position, int &Dim_x, int &Dim_y)
{
    cout << "Lets create the game map." << endl;
    cout << "Enter number of Columns: ";
    cin >> Dim_x;
    cout << "Enter number of Rows: ";
    cin >> Dim_y;

    Position = new GameGrid[Dim_x * Dim_y]

    for (int y=0; y < Dim_y; y++)
    {
        cout << setw (20);

        for (int x=0; x < Dim_x; x++)
        {
            cout << Position[x*Dim_y + y].Element;
            cout << char(32);
        }
        cout << endl;
    }
}

int main()
{
    struct GameGrid *Position = NULL;
    int Dim_x;
    int Dim_y;

    CreateMap(Position, Dim_x, Dim_y);

    delete[] Position;
    return 0;
}

The Grid should look something like this for dimensions Dim_x=6 and Dim_y=6 (chosen by end-user).

A A A A A A
A A A A A A
A A A A A A
A A A A A A
A A A A A A
A A A A A A

Also when the printing the grid is done two times (once in the function CreateMap and once in main), it prints them both times, then freezes for 10 secs and dies.

Upvotes: 2

Views: 64

Answers (2)

Baxter
Baxter

Reputation: 126

Although you're using a pointer as a parameter, you are still passing-by-value.

When trying to pass-by-reference the pointer itself is not changed but the object at the address it points to is updated.

So to update the pointer you need a pointer to the pointer or a reference to a pointer. Otherwise, often as has been answered, the return is used to return an updated pointer.

Upvotes: 0

tadman
tadman

Reputation: 211740

In your CreateMap function:

void CreateMap (struct GameGrid *Position, int &Dim_x, int &Dim_y)
{
    // ...    
    Position = new GameGrid[Dim_x * Dim_y];
}

This modifies the Position local variable only, it does not alter the caller's value supplied to that argument.

What you need is to re-work this:

GameGrid *CreateMap(const int Dim_x, const int Dim_y)
{
    // ...    
    return new GameGrid[Dim_x * Dim_y];
}

Where that returns a value you can capture:

int main()
{
  int x, y;
  cout << "Lets create the game map." << endl;
  cout << "Enter number of Columns: ";
  cin >> x;
  cout << "Enter number of Rows: ";
  cin >> y;

  GameGrid *Position = CreateMap(x, y);
  // ...
}

Do all your input/output outside of these functions. Remember your SOLID Principles, give that function one job and one job only. Input and allocation are two jobs.

Even better: Make a constructor instead of these CreateX functions. This is C++ and you can take full advantage of that.

As always:

Whenever you're having strange behaviour, step through your code in a debugger to see what the values of various variables are as it executes.

Upvotes: 2

Related Questions