Why can I not access this array whilst implementing an initializer list constructor for a Matrix class?

Question

I'm working with an old code base that implements a matrix. I'm trying to add a method to enable construction from an initializer list. I'm quite confused because this was working yesterday. I'm not sure what I've changed by this code crashes and burns when it gets to _Array[row * col + col] = r.begin()[col];. Why can I not access this _Array?

Heres an abstracted example:

#include <initializer_list>
#include <iostream>

template<class T>
class Matrix {

    void resize(unsigned int rows, unsigned int cols) {
        if (rows * cols != _Rows * _Cols) {
            if (_Array) {
                delete[] _Array;
                _Array = nullptr;
            }
            if (rows && cols) {
                _Array = new T[rows * cols];
            }
        }

        _Rows = rows;
        _Cols = cols;
    }

    /**
     * number of rows
     */
        unsigned int _Rows;

    /**
     * number of columns
     */
    unsigned int _Cols;

    /**
     * pointer to block of memory of the data, stored in row-major format.
     */
    T *_Array;

public:
    Matrix<T>(std::initializer_list<std::initializer_list<T>> matrix)
        : _Array(nullptr) {
        _Rows = matrix.size();
        _Cols = (*matrix.begin()).size();
        resize(_Rows, _Cols);

        for (int row = 0; row < _Rows; row++) {
            const std::initializer_list<T> &r = matrix.begin()[row];
            for (int col = 0; col < _Cols; col++) {
                printf("Row: %d; col: %d; value: %d", row, col, r.begin()[col]);
                _Array[row * col + col] = r.begin()[col];
            }
        }
    }
};

int main(){
    Matrix<int> matrix(
            {
                    {1, 2, 3},
                    {4, 5, 6},
            });
}

clion output:

AddressSanitizer:DEADLYSIGNAL
Row: 0; col: 0; value: 1
Process finished with exit code 6

P.S. I would love to just use STL types but this is not an option and I'm aware of the bad practice of prepending variable names with underscores.

Your attempt to resize the matrix at the start of the constructor fails. Because the resize code is written not to reallocate if the new size is equal to the old size, but that is exactly what you are doing. So `_Array` is still null when you start to fill your matrix. — john, Sep 23 '20 at 13:25
@Eljay Why not? When I comment out the line causing the segfault I get the followin output: `Row: 0; col: 0; value: 1 Row: 0; col: 1; value: 2 Row: 0; col: 2; value: 3 Row: 1; col: 0; value: 4 Row: 1; col: 1; value: 5 Row: 1; col: 2; value: 6` — CiaranWelsh, Sep 23 '20 at 13:29
@john I thought of this too, but the number of `_Rows` is 2 and `_Cols` is 3 which `!=` the null initialized Array. What am I missing? — CiaranWelsh, Sep 23 '20 at 13:31
Oooh - gotchya. It should be `resize(matrix.size(), (*matrix.begin()).size());`. Thanks for the hints. Happy to accept an answer if you provide one. — CiaranWelsh, Sep 23 '20 at 13:35
*"...I'm aware of the bad practice of prepending variable names with underscores."* That's not a bad practice as long as it is a member variable, and it starts with an underscore and lowercase letter. (Starting with underscore and uppercase letter is asking for trouble, since all those identifiers are **reserved**.) — Eljay, Sep 23 '20 at 13:35
[The Underscore Diaries](https://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier). — user4581301, Sep 23 '20 at 13:36
Change your trace output to `printf("_Array[%d] is Row: %d; col: %d; value: %d\n", row * col + col, row, col, r.begin()[col]);` to see what I meant earlier. — Eljay, Sep 23 '20 at 13:38
Oh you already have it covered in comments. I was busy writing and checking my answer, at the same time. Nice question with a good [mcve], btw -- my compliments. — Kenny Ostrom, Sep 23 '20 at 13:39

score 1 · Accepted Answer · answered Sep 23 '20 at 13:38

You initialize everything to an invalid state

_Array = nullptr
_Rows = matrix.size();
_Cols = (*matrix.begin()).size();

This is incorrect because your code assumes that _Rows and _Cols are accurate metadata about _Array.

You then ask resize to do nothing

resize(_Rows, _Cols);

void resize(unsigned int rows, unsigned int cols) {
    if (rows * cols != _Rows * _Cols) {
        // unreachable code
    }
    // redundant you already set these
    _Rows = rows;
    _Cols = cols;
}

Consequently _Array will always be nullptr, which you then dereference by attempting to use it as an array pointer.

Why can I not access this array whilst implementing an initializer list constructor for a Matrix class?

1 Answers1