Vector assignment crashing

问题:

vector< vector<int> > resizeVector(vector< vector<int> > m)
{
    vector< vector<int> > newMatrix;
    int i,j;

    for (i = 0; i < m[i].size(); i++)
    {
        for(j = 0; j < m[j].size(); j++)
        {
            newMatrix[i][j] = m[i][j];
        }
    }
    return (newMatrix);
}

I am making a program that will do a whole lot of matrix manipulation, and this section is crashing and I don't exactly know why. I have narrowed it down to the line:

newMatrix[i][j] = m[i][j];

It crashes right here, and I am not sure why.

回答1:

In addition to what @Saurav posted, newMatrix is empty so you cannot assign values to newMatrix[i][j]. You can fix this by initializing the vectors with a given size:

vector< vector<int> > resizeVector(vector< vector<int> > m)
{
    vector< vector<int> > newMatrix(m.size());
    int i,j;

    for (i = 0; i < m.size(); i++)
    {
        newMatrix[i].resize(m[i].size());
        for(j = 0; j < m[i].size(); j++)
        {
            newMatrix[i][j] = m[i][j];
        }
    }
    return (newMatrix);
}

Before the for-loops we initialize newMatrix to have m.size() many empty vectors inside of it (the vectors are empty due to their default constructor). During each iteration of the outer for-loop we ensure that each vector within newMatrix has the correct size using the resize member function.

Note that if you want a copy of a vector you can simply just write:

vector< vector<int> > newMatrix(m);

回答2:

Your checks are wrong. Instead it should be

for (i = 0; i < m.size(); i++)        // m.size() gives the number of rows 
{
    for(j = 0; j < m[i].size(); j++)  // m[i].size() gives the number of columns in the row

回答3:

You're assigning into your new newMatrix without setting its size first. It will default to empty, and any attempt to assign to it will result in undefined behavior.

Since you don't pass in a new size, it's hard to know exactly what you're trying to accomplish. That's why I don't have more explicit advice on how to fix it.

回答4:

If you want to allocate vector of vector, the you need to allocate memory for the matrix before you index on it. So you will have to use something like

newMatrix.resize(size);
for (int i = 0; i < size; ++i) {
    newMatrix[i].resize(size);
}

Or you could use .push_back() vector method to add values to the vector without allocating memory beforehand.

回答5:

vectors operator[] returns a reference to the specified element without bounds checking.

That means it does not magically resize the vector, or perform any other operations to ensure the element exists. If the element does not exist, the result is undefined behaviour - which means means anything can happen. Practically, it often causes the program to access an invalid memory location, which causes a crash.

The above is true even for a simple vector<int>. You've compounded your problems by working with a vector<vector<int> > (Each element of a vector<vector<int> > is a vector<int>).

Your function is actually a little breathtaking in the number of times it potentially invokes undefined behaviour. You happen to be getting a crash on the statement newMatrix[i][j] = m[i][j], but the potential for undefined behaviour actually occurs before then.

In the outer loop, m[i] will not exist if m.size() (which your code doesn't check) has a value zero. If m.size() is zero, that causes the evaluation of m[i] (as m.operator[](i)) to have undefined behaviour.

Essentially, you need to ensure any index i is valid before evaluating m[i], and then ALSO ensure that j is a valid index of m[i] before evaluating m[i][j]. And then do the same for newMatrix. Your code does none of this at all. A more correct rendering of your function (assuming the intent is to create a copy of m) is

vector< vector<int> > resizeVector(vector< vector<int> > m)
{
    vector< vector<int> > newMatrix;
    int i,j;

    newMatrix.resize(m.size());    //  necessary to ensure we can access newMatrix[i] in the loop below

    for (i = 0; i < m.size(); i++)     //   ensure i is a valid index of m
    {
        // newMatrix[i].size() is still zero, so we need to resize for use in the inner loop

        newMatrix[i].resize(m[i].size()); 

        for(j = 0; j < m[i].size(); j++)   // ensure j is a valid index of m[i]
        {
            newMatrix[i][j] = m[i][j];
        }
    }
    return (newMatrix);
}

Now, the thing is, your code is actually recreating functionality that vector provides already. So, we could replace the body of the function quite simply with

vector< vector<int> > resizeVector(vector< vector<int> > m)
{
      vector< vector<int> > newMatrix(m);  
      return newMatrix;
}

or even with

vector< vector<int> > resizeVector(vector< vector<int> > m)
{
      return m;
}

This means your function (as I've modified it) is misnamed - it doesn't resize anything. In fact, it is rather pointless since, if the caller does this

x = resizeVector(y);

it could achieve the same effect without your function at all, simply as

x = y;

which is also more efficient (no function call, no creating copies to pass by value, etc).