0

I'm new to pointers and I'm trying to implement a simple code, in which I read and write a 2D array, yet I have no idea why this isn't working. Would anyone provide any suggestions?

#include<stdio.h>
#include<stdlib.h>

int read_matr(int ***m, FILE *f){
    int i,j,length,a;
    fscanf(f,"%d",&length);
    *m = (int **)malloc(length*sizeof(int));
    for(i=0;i<length;++i)
        for(j=0;j<length;++j){
                fscanf(f,"%d",&a);
                *(*(m+i)+j) = a;
        }
    return length;
}

void write_matr(int *m[][3], int length, FILE *g){
    int i,j;
    for(i=0;i<length;++i){
        for(j=0;j<length;++j)
            fprintf(g,"%d ",*(*(m+i)+j));
        fprintf(g,"\n");
    }
    fprintf(g,"\n");
}

int main(){
    FILE *f = fopen("datein.txt","r");
    FILE *g = fopen("dateout.txt","w");
    int **m, n = read_matr(m,f);
    write_matr(m,n,g);
    free(m);
    return 0;
}
theSongbird
  • 229
  • 1
  • 3
  • 13
  • 2
    What isn't working? What is the expected behaviour? What behaviour do you get vs. expected behaviour? – Baldrick Jan 27 '17 at 14:48
  • Ah yes, the daily three star programming, with the mandatory answers by other three star programmers. I think I'm gonna write a FAQ community wiki about how to do this proper, because very few C programmers seem to know it. – Lundin Jan 27 '17 at 15:26

4 Answers4

2

To correctly allocate a 2D array in modern C, you do like this:

int (*array)[x][y] = malloc( sizeof(int[x][y]) );

which could be simplified for convenience as:

int (*array)[y] = malloc( sizeof(int[x][y]) );

To correctly allocate a 2D array in ancient versions of C (C90/ANSI-C), you would use "mangled arrays":

int* array = malloc(x*y*z*sizeof(int));

The above are the only ways to dynamically allocate a true 2D array in C. Anything based on pointer-to-pointers is not a 2D array, nor can it be used as one. This is because such pointer-to-pointer "things" are scattered all over the heap. They are slow and needlessly complex.

Lundin
  • 195,001
  • 40
  • 254
  • 396
1

I suggest you should look at the compiler warnings. There are a lot in this code. With the help of these you should be able to solve some things on your own.

What i don't get. You define int **m and give it uninitialized to the function read_matr(int ***m, FILE *f) but what you need to to in that case would be read_matr(&m, f);

EDIT

So here are just some mistakes you made.

  1. Please do not cast the pointer malloc returns.

    int **m;
*m = malloc(length*sizeof(int));

Be aware, that what you get is a pointer to a pointer to allocated memory. This is not equal to m[][]. If you want to allocate a 2D array your way you should do something like this:

int **m;
int rows, cols;
*m = malloc(sizeof(rows) * rows);
for(int i = 0; i < rows; i++){
    m[i] = malloc(sizeof(cols) * cols);
}

See: Using malloc for allocation of multi-dimensional arrays with different row lengths

Community
  • 1
  • 1
bitflip-at
  • 149
  • 2
  • 14
1

There are at least two problems with the provided code. :

  • You are passing a **int variable as first parameter of read_matr, but this function is requesting a ***int parameter (a pointer to a 2dimensional array). The parameter seems correct, but then you need to pass a pointer to your array in this way:

    int n = read_matr(&m, f);

  • Your array is supposed to be 2d dimensional, but you are only allocating it as a 1dimensional array. After allocating a first array, you need to allocate each row independently:

    *m = (int **) malloc(length * sizeof(int*));
for(i=0; i<length; ++i) {
    (*m)[i] = (int*) malloc(length * sizeof(int));
    ...
}

Optionally, you can make a 1dimensionnal array of size length*length, and then you would access m[i][j] by m[i * length + j]

Warnings could have helped you a great deal there.

Rémi Bonnet
  • 793
  • 5
  • 16
0

My first suggestion would be to separate memory management from I/O. First create your matrix, and once you've verified that succeeded, then read your input.

My second suggestion would be to use regular subscript notation (m[i][j]) instead of explicit pointer dereferences (*(*(m + i) + j)). Easier to read, harder to get wrong.

Finally, when you allocate memory this way, you have to allocate memory for each row individually:

void createMatrix( int ***m, size_t length )
{
  *m = malloc( sizeof **m * length );
  if ( !*m )
  {
    // memory allocation error, bail out here
    return;
  }

  size_t i;
  for ( i = 0; i < length; i++ )
  {
    /**
     * Postfix [] has higher precedence than unary *, so *m[i] will be
     * parsed as *(m[i]) - it will index into m and dereference the result.
     * That's not what we want here - we want to index into what m 
     * *points to*, so we have to explicitly group the unary * operator
     * with m and index into the result.
     */
    (*m)[i] = malloc( sizeof *(*m)[i] * length );
    if ( !(*m)[i] )                              
      break;
  }

  if ( i < length )
  {
    // memory allocation failed midway through allocating the rows;
    // free all previously allocated memory before bailing out
    while ( i-- )
      free( (*m)[i] );
    free( *m );
    *m = NULL;
  }
}

Assuming you called it as

int **m;
createMatrix( &m, 3 ); // note & operator on m

you wind up with something that looks like this:

     +---+                                                  +---+
m[0] |   | ---------------------------------------> m[0][0] |   |
     +---+                                +---+             +---+
m[1] |   | ---------------------> m[1][0] |   |     m[0][1] |   |
     +---+               +---+            +---+             +---+
m[2] |   | ----> m[2][0] |   |    m[1][1] |   |     m[0][2] |   |
     +---+               +---+            +---+             +---+
      ...        m[2][1] |   |    m[1][2] |   |              ...
                         +---+            +---+
                 m[2][2] |   |             ...
                         +---+
                          ...

This is not a true 2D array - the rows are not contiguous. The element immediately following m[0][2] is not m[1][0]. You can index it as though it were a 2D array, but that's about it.

Sometimes that's not the wrong answer. Depending on how fragmented your heap is, a single NxM allocation request may fail, but N separate allocations of M may succeed. If your algorithm doesn't rely on all elements being contiguous then this will work, although it will likely be slower than using a true NxM array.

If you want a true, contiguous 2D array whose dimensions are not known until runtime, then you will need a compiler that supports variable-length arrays (VLAs), and you would allocate it as follows:

size_t length = get_length_from( f );
int (*m)[length] = malloc( sizeof *m * length ); // sizeof *m == sizeof (int [length])

m is a pointer to a length-element array of int, and we're allocating enough memory for length such arrays. Assuming length is 3 again, you wind up with something that looks like this:

        +---+
m[0][0] |   |
        +---+ 
m[0][1] |   |
        +---+ 
m[0][2] |   |
        +---+ 
m[1][0] |   |
        +---+ 
m[1][1] |   |
        +---+ 
m[1][2] |   |
        +---+ 
m[2][0] |   |
        +---+ 
m[2][1] |   |
        +---+ 
m[2][2] |   |
        +---+

If VLAs are not available (versions earlier than C99, or C2011 where __STDC_NO_VLA__ is defined) and you want all your array elements to be contiguous, then you'll have to allocate it as a 1D array:

size_t length = 3;
int *m = malloc( sizeof *m * length * length );

You can set up an additional array of pointers to pretend m is a 2D array:

int *q[] = { &m[0], &m[3], &m[6] };

So

q[0][0] == m[0];
q[0][1] == m[1];
q[0][2] == m[2];
q[1][0] == m[3];
q[1][1] == m[4];
...

etc.

John Bode
  • 119,563
  • 19
  • 122
  • 198