Crash with 2D C-style arrays

Question

A common problem with 2D C-style arrays. This program compiles, but crashes when the Print() function is called:

#include <iostream>
using namespace std;

static const int sx = 5, sy = 3;
static double M[sy][sx] = { { 0.0, 1.0, 2.0, 3.0, 4.0 },
                            { 0.1, 1.1, 2.1, 3.1, 4.1 },
                            { 0.2, 1.2, 2.2, 3.2, 4.2 } };

void Print(double **M, int j, int i)
{
    cerr << M[j][i] << endl;
}

int main()
{
    cerr << M[2][3] << endl;             // Prints 3.2
    Print((double**)&M[0][0], 2, 3);     // Crash
}

What is wrong in my code?

Is it possible to make it work without changing Print()'s signature?

The array is not dynamic.

Answer 1

As someone else already pointed out &M[0][0] is a double * because you are just retrieving the address of the element M[0][0]. If you cast it as a double ** and then try to stride in memory pretending it really is a double ** you are basically looking for troubles. What I guess you are trying to do can be safely achieved by explicitly doing a two-layer allocation, i.e. something like:

double ** M = new double*[sy];

for (int iy=0; iy<sy; iy++) {
    M[iy] = new double[sx];
}  

then (after initialization) Print(M, 2, 3); will work fine. I'm afraid you'll have to give up on the array-style initialization with the curly-bracket notation if you do it this way and look for some way around that. (NOTE: my suggestion above is far from efficient for large arrays, as the various 1D chunks of the 2D array are not guaranteed to be allocated contiguously. In general, if size and performance are issues you should consider representing 2D arrays as 1D arrays.)

Answer 2

The array is not dynamic

Is it possible to make it work without changing Print()'s signature?

Nope, it's not possible to achieve both requirements. Unless it's ok for you to copy that 2D array to another which is dynamic.

A 2D array of doubles is not an array of pointers to the rows. Multidimensional arrays are flat as far as memory is concerned. The type of M[0][0] is double and therefore the type of &M[0][0] is *double.

See this tutorial on using arrays. Pay attention to the chapter "Multidimensional arrays".

What you probably meant to do could be done like this:

void Print(double *M, int j, int i, int width) {
    cerr << M[i + width * j] << endl;
}

Print(&M[0][0], 2, 3, sx);

Note: As chris commented, this is not technically allowed. Another reference.

It's possible to do your function and be technically correct, but that requires the function to be particular to the width of the array. Since you use C++, it's not a problem because you can use a template:

template<size_t X>
void Print(double M[X], int i) {
    cerr << M[i] << endl;
}

 Print<sx>(M[2], 3);

A side note: I don't believe that this one-liner is worthy of being a function. In my opinion it reduces readability and doesn't improve reusability.

Answer 3

It is possible to make the Print function work, but you need to change the way you allocate the array. Specifically you need a vector of pointers to the first element of each row, and pass that as the argument.

Here is how you might do that:

#include <iostream>
#include <cstdlib>
using namespace std;

static const int sx = 5, sy = 3;
static double M[sy][sx] = { { 0.0, 1.0, 2.0, 3.0, 4.0 },
                            { 0.1, 1.1, 2.1, 3.1, 4.1 },
                            { 0.2, 1.2, 2.2, 3.2, 4.2 } };

double **twoD(int nr, int nc) {
// function that allocates a 2D matrix and returns the result
// as a vector of pointers to the first element of each row
  double **p;
  int ii;
  p = (double**)malloc(nr * sizeof(double*));
  p[0] = (double*)malloc(nr * nc * sizeof(double));
  for(ii=1; ii<nr; ii++) {
    p[ii] = p[0] + ii * nc;
  }
  return p;
}

void free2d(double **p) {
  free(p[0]);
  free(p);
}

void Print(double **M, int j, int i)
{
    cerr << M[j][i] << " ";
}

int main()
{
    double **P;
    P = twoD(sy, sx);
    memcpy(P[0], M, sx * sy * sizeof(double));
    cerr << M[2][3] << endl;             // Prints 3.2
    int ii,jj;
    for(ii=0; ii<sy; ii++) {
      for(jj=0; jj<sx; jj++) {
        Print(P, ii, jj);
      }
      cerr << endl;
    }
    free2d(P);
}

As you can see - it doesn't change the signature of the Print() function, yet allows you to use it without crashing.

I'm not saying I recommend this as the best approach to take - but it answers the question literally. Note I took out the endl from Print just so that I could convince myself that this would print out the entire array correctly.

This is adapted from nrutil.c from "Numerical Recipes in C" (see for example source code here - this is how that book creates variable length 2D arrays (actually they go one step further and allow you to use base 1 offsets - which I find horrific but makes the code "look like" FORTRAN which was the original language of scientific computing).

Answer 4

Arrays and pointers are different types. An array is implicitly converted (decays) into a pointer to its first element in some situations like in this case when the array is passed to the print function.

The following statement

static double M[sy][sx] = { { 0.0, 1.0, 2.0, 3.0, 4.0 },
                            { 0.1, 1.1, 2.1, 3.1, 4.1 },
                            { 0.2, 1.2, 2.2, 3.2, 4.2 } };

defines M to be an array of sy, i.e., 3 elements where each element is of type double[5], i.e., an array of 5 doubles. Therefore, in the print function call

Print((double**)&M[0][0], 2, 3);

&M[0][0] evaluates to the address of M[0][0] which is of type (double *). Casting this value to type (double **) is wrong because the value at this address is a double, not a pointer to a double.

Now, in the print function, M[i][j] is evaluated to *(*(M + i) + j). Here, the variable M (local to the print function) is the address of M[0][0]. Therefore, *(M + i) evaluates to M[0][i] which is a double. Next, j is added to this value and assuming this value to be a valid memory address, the value at that address is fetched. This is plain wrong and invokes undefined behaviour.

You need to change the signature of print function.

#include <iostream>
using namespace std;

static const int sx = 5, sy = 3;
static double M[sy][sx] = { { 0.0, 1.0, 2.0, 3.0, 4.0 },
                            { 0.1, 1.1, 2.1, 3.1, 4.1 },
                            { 0.2, 1.2, 2.2, 3.2, 4.2 } };

void Print(double *M, int j, int i)
{
    cerr << M[j*sx + i] << endl;
}

int main()
{
    cerr << M[2][3] << endl;             // Prints 3.2
    Print(&M[0][0], 2, 3);    
}

Answer 5

This:

static double M[sy][sx] = { { 0.0, 1.0, 2.0, 3.0, 4.0 },
                            { 0.1, 1.1, 2.1, 3.1, 4.1 },
                            { 0.2, 1.2, 2.2, 3.2, 4.2 } };

is nothing remotely like a double**, and casting it or anything within it to such a type is incorrect. If you're using a fixed arrays of arrays (call it a 2D array if you want), a compliant print function can be crafted using a template:

#include <iostream>
using namespace std;

static const int sx = 5, sy = 3;
static double M[sy][sx] = { { 0.0, 1.0, 2.0, 3.0, 4.0 },
                            { 0.1, 1.1, 2.1, 3.1, 4.1 },
                            { 0.2, 1.2, 2.2, 3.2, 4.2 } };


template<int M>
void Print( double(*ar)[M] , int j, int i )
{
    std::cout << ar[j][i] << '\n';
}

int main()
{
    Print(M, 2, 3);
    return 0;
}

Answer 6

The problem is &M[0][0] is NOT a double **, it is only a pointer to a double (double *). You need to change the function prototype to typedef double my_array_type [3][5]; void Print (my_array_type *M, int j, int i)