Magic Square in C++

Question

Yet another magic square problem. I'm creating an odd magic square program in C++, and for some reason the program keeps giving a segmentation fault error and quitting. Here is the code:

#include <iostream>

using std::cin;
using std::cout;

#include <cstring>

using std::memset;

int *generateOddSquare(int n) {
    if (n % 2 != 0 && n >= 3) {
        int row = 0, col = n / 2, square = n * n;
        int **matrix = new int *[n], *dest = new int[square];

        memset(matrix, 0, sizeof(matrix[0][0]) * square);

        for (int i = 1; i <= square; i++) {
            matrix[row][col] = i;

            if (i % n == 0)
                row++;
            else {
                if (row == 0)
                    row = n - 1;
                else
                    row--;

                if (col == (n - 1))
                    col = 0;
                else
                    col++;
            }
        }

        for (int i = 0; i < n; i++) {
            for (int j = 0; j < n; j++) {
                dest[(i * n) + j] = matrix[i][j];
            }
        }

        return dest;
    } else
        return NULL;
}

int main() {
    int *arr = generateOddSquare(3);

    for (int i = 0; i < 9; i++) {
        cout << arr[i] << "\n";
    }
}

What is wrong with it? Is the way I'm declaring my pointers correct?

You haven't initialised the matrix row pointers. – Paul R Apr 14 '15 at 15:25 — Paul R, Apr 14 '15 at 15:25

score 2 · Accepted Answer · answered Apr 14 '15 at 15:27

You create an array of pointers:

int **matrix = new int *[n]

but don't initialise those to point to anything; hence the segementation fault when you try to dereference them. If you really must juggle pointers, then allocate an array for each to point at:

for (int i = 0; i < n; ++i) {
    matrix[i] = new int[n];
}

and don't forget to delete all of these allocations, if you care about memory leaks.

Unless this is an exercise in masochism, use the standard library to make life easier:

std::vector<std::vector<int>> matrix(n, std::vector<int>(n));

and return std::vector<int> rather than int* to save the caller the hassle of juggling and deleting the pointer.

Ya, I did that, and then simply flattened the matrix and returned the vector's data, but that didn't end up working out well enough since I couldn't find a significant way to iterate over them. I fixed that though so I may switch back to them. — T145, Apr 14 '15 at 15:34

score 1 · Answer 2 · answered Apr 14 '15 at 15:27

You are only partially instantiating matrix. You have int **matrix = new int *[n] which will give you your rows but you are defining the columns. To completely initialize you need to use

int **matrix = new int *[n];
for (int i = 0; i < col_dimension; i++)
    matrix[i] = new int[col_dimension];

score 1 · Answer 3 · answered Apr 14 '15 at 15:27

1

You are dereferencing null pointers. You have a 2-d array:

int **matrix = new int *[n];

That you clear (incorrectly - size should be should be n * sizeof(*matrix)):

memset(matrix, 0, sizeof(matrix[0][0]) * square);

And then immediately write into:

for (int i = 1; i <= square; i++) {
    matrix[row][col] = i;
    ....
}

But matrix[0] is NULL. You need to allocate all of the pointers first!

for (int i = 0; i < n; ++i) {
    matrix[i] = new int[whatever];
}

answered Apr 14 '15 at 15:27

Barry

286,269
29
621
977

Would `whatever` be `n`? – T145 Apr 14 '15 at 15:38
@T145 If it's supposed to be square, then yes. – Barry Apr 14 '15 at 15:45

score 0 · Answer 4 · answered Sep 05 '19 at 19:33

use vectors.

    vector<vector<int> > matrix(n, vector<int>(n, 0));
    OddMagicSquare(matrix, n, -1);



void OddMagicSquare(vector<vector<int>> &matrix, int n)
{
    auto nsqr = n * n;

    // start position
    auto row = rand() % n;
    auto col = rand() % n;
    auto start = 1;

    for (auto index = start; index <= nsqr + (start -1); ++index)
    {
        while (col >= n)
            col -= n;

        while (col < 0)
            col += n;

        while (row >= n)
            row -= n;

        while (row < 0)
            row += n;

        matrix[row][col] = index;

        row--;
        col++;

        if (index%n == 0)
        {
            row += 2;
            --col;
        }
    }
}

Magic Square in C++

4 Answers4