Multidimensional array element assignment with pointer

Question

I have a jagged 2D array of struct and a function that allocate and return mystruct*:

mystruct** arr;
mystruct* foo();

What is the syntax for assignment of element of arr? The following doesn't work but I want something like it:

&arr[i][j]=foo();

Here is a simplified example:

typedef struct mystruct{
    int a,b,c,d;
} mystruct;

mystruct* foo()
{
    mystruct* x = (mystruct*)malloc(sizeof(mystruct));
    x->a=1;
    x->b=1;
    x->c=1;
    x->d=1;
    return x;
}

int main()
{
    int rows=3;
    int cols=4;

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

    for (int i=0;i<rows;i++)
    {
        *(arr[i][0]) = foo();
    }

    return 0;
}

I've stripped off most of my main program, but the code above still gives me error: no match for ‘operator*’ (operand type is ‘mystruct’)

In the real code mystruct is big and the array has big dimensions.

`mystruct** arr[3][4];` is a 2-d array of *pointers* (NOT a 2-d array of struct). Those are pointers to pointers to struct. Are you sure that is what you want? Maybe you meant `mystruct* arr[3][4];` and then `arr[i][j] = foo();` — M.M, Feb 17 '15 at 00:15
@MattMcNabb I think that's what I want. What are the alternatives? — nullgraph, Feb 17 '15 at 00:18
@swang that will only work if `arr[i][j]` has previously been pointed to valid storage for a `mystruct *` — M.M, Feb 17 '15 at 00:19
@MattMcNabb I edited the language to C. Pretty sure I want mystruct** arr[3][4], but would appreciate some thoughts on pros/cons vs mystruct* arr[3][4]. — nullgraph, Feb 17 '15 at 00:23
@nullgraph we can't say without knowing more about what problem you are trying to solve. It does seem very strange to have an array of 3x4 pointers each to a single malloc'd pointer, each of which then points to a malloc'd instance though. It would be simpler just to have an array of 12 instances. Whether or not that works for your problem or not we can't say yet. — M.M, Feb 17 '15 at 00:34
@MattMcNabb fair point, the arrays are actually quite huge and the struct more complicated, I agree it's hard to give a general statement. — nullgraph, Feb 17 '15 at 00:49
In the updated code (which is completely different to your original description) you have too many allocations. You already made the rows contain `mystruct`. It's not possible to redirect a single element of a row to "point" somewhere else. You could copy the struct returned by `foo` and `free()` the pointer returned by `foo()`, but then it was a waste of time calling `malloc` in `foo` in the first place. — M.M, Feb 17 '15 at 00:50
I recognize the question is faulty. Does anyone know what I should do in this case? Do I accept the answer? — nullgraph, Feb 17 '15 at 00:58
Do you need each row of the array to be able to be different lengths? Is it intentional to initialize column 0 and leave the other columns be uninitialized pointers? — M.M, Feb 17 '15 at 01:00
No, all rows of the array have the same length. I didn't put in the code for initializing other columns, but they are initialized by a different function void bar(mystruct* x). I changed foo() to follow the same pattern. — nullgraph, Feb 17 '15 at 01:04
@nullgraph `bar` works differently than `foo` in that it doesn't do its own allocation, this is an important detail — M.M, Feb 17 '15 at 01:06
@MattMcNabb Yes, I realized that and changed foo() after your comment about too many allocations. — nullgraph, Feb 17 '15 at 01:07

score 1 · Accepted Answer · edited May 23 '17 at 12:20

If you really want all of the instances of mystruct to each be in a single allocation (i.e. leave foo unchanged) then the array could be:

int main()
{
    int rows=3;
    int cols=4;

    mystruct *(*arr)[cols] = malloc( rows * sizeof *arr );

    for(int i=0; i<rows; i++)
        arr[i][0] = foo();

    // note: arr[i][1] etc. are uninitialized 

    free(arr);
}

There is no need to use a separate malloc for each row of an array when every row is the same length, instead you can malloc a single chunk. And don't cast malloc.

If you do not need a single allocation per line, and you are happy to construct mystruct with void bar(mystruct *p) that is called with a pointer to some allocated storage, then your array can store mystruct objects directly:

mystruct (*arr)[cols] = malloc( rows * sizeof *arr );

for ( int i = 0; i < rows; ++i )
    bar( &arr[i][0] );

One more question: how do I deallocate the array if it's allocated that way? — nullgraph, Feb 17 '15 at 01:32

score 0 · Answer 2 · answered Feb 17 '15 at 00:24

0

*(arr[i][j]) = foo(); will set value of pointer to pointer to struct in 2d array to pointer to struct returned by foo, if that is what you really want.

If you just want to keep 2d array of pointers to struct, then you would change array declaration to mystruct* arr[3][4] and use arr[i][j] = foo().

answered Feb 17 '15 at 00:24

Martinsos

1,663
15
31

Still getting error "no match for 'operator*' for *(arr[i][j]) = foo(); – nullgraph Feb 17 '15 at 00:27
@nullgraph Could you show piece of code where that happens, together with some surrounding code, and exact line of error? – Martinsos Feb 17 '15 at 00:30
@nullgraph that is the error message you would get if `arr` was actually `mystruct arr[i][j];` (or `mystruct **arr;` etc.) – M.M Feb 17 '15 at 00:37

user1732602 · Answer 3 · 2015-02-17T18:25:06.883

I think that using a few abstractions would help in this scenario. I wrote the following example in a rush, but I hope it helps.

Update: Changed according to comments. Still compiles under gcc and now cleanly runs through Valgrind.

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

typedef struct SomeStruct SomeStruct;
struct SomeStruct {
  int foo;
  int bar;
};
typedef SomeStruct* SomeStruct_ptr;
typedef struct SomeStructMat SomeStructMat;
typedef SomeStructMat* SomeStructMat_ptr;
struct SomeStructMat {
  int rows;
  int cols;
  SomeStruct_ptr** data;
};

SomeStruct_ptr somestruct_new(int foo, int bar) {
  SomeStruct_ptr ssp = malloc(sizeof(SomeStruct));
  ssp->foo = foo;
  ssp->bar = bar;
  return ssp;
}

void somestruct_destroy(SomeStruct_ptr ssp) {
  free(ssp);
}

SomeStruct_ptr** somestructmat_new(int rows, int cols) {
  int r, c;
  SomeStruct_ptr** mat = malloc( rows * sizeof(SomeStruct_ptr*) );

  for(r = 0; r < rows; ++r) {
    mat[r] = malloc(sizeof(SomeStruct_ptr) * cols);
    for(c = 0; c < cols; ++c) {
      mat[r][c] = somestruct_new(0,0);
    }
  }
  return mat;
}

SomeStructMat_ptr SomeStructMat_new(int rows, int cols) {
  SomeStructMat_ptr mat = malloc(sizeof(SomeStructMat));
  mat->rows = rows;
  mat->cols = cols;
  mat->data = somestructmat_new(rows, cols); 
  return mat;
}

void SomeStructMat_destroy(SomeStructMat_ptr mat) {
  int r,c;
  for(r = 0; r < mat->rows; r++) {
    for(c = 0; c < mat->cols; c++) {
      free(mat->data[r][c]);
    }
    free(mat->data[r]);
  }
  free(mat->data);
  free(mat);
}

int SomeStructMat_overwrite(SomeStructMat_ptr ssm, int r, int c, SomeStruct_ptr ssp) {
  // TODO: Check that r and c are within bounds!
  somestruct_destroy(ssm->data[r][c]);
  ssm->data[r][c] = ssp;
}

void SomeStructMat_print(SomeStructMat_ptr mat) {
  int r,c;
  for(r = 0; r < mat->rows; r++) {
    for(c = 0; c < mat->cols; c++) {
      printf("[%d, %d] ", mat->data[r][c]->foo, mat->data[r][c]->bar);
    }
    printf("\n");
  }
}

int main(int argc, char** argv) {
  SomeStructMat_ptr ssm = SomeStructMat_new(2,3);

  SomeStructMat_print(ssm);

  printf("----------------\n");

  SomeStructMat_overwrite(ssm, 0, 1, somestruct_new(5,6));

  SomeStructMat_print(ssm);

  printf("----------------\n");

  SomeStructMat_destroy(ssm);

  return 0;
}

Update: To compile under g++, you'll need to cast the void pointers coming out of malloc. Like so:

#include <cstdio>
#include <cstdlib>

typedef struct SomeStruct SomeStruct;
struct SomeStruct {
  int foo;
  int bar;
};
typedef SomeStruct* SomeStruct_ptr;
typedef struct SomeStructMat SomeStructMat;
typedef SomeStructMat* SomeStructMat_ptr;
struct SomeStructMat {
  int rows;
  int cols;
  SomeStruct_ptr** data;
};

SomeStruct_ptr somestruct_new(int foo, int bar) {
  SomeStruct_ptr ssp = (SomeStruct_ptr) malloc(sizeof(SomeStruct));
  ssp->foo = foo;
  ssp->bar = bar;
  return ssp;
}

void somestruct_destroy(SomeStruct_ptr ssp) {
  free(ssp);
}

SomeStruct_ptr** somestructmat_new(int rows, int cols) {
  int r, c;
  SomeStruct_ptr** mat = (SomeStruct_ptr**) malloc( rows * sizeof(SomeStruct_ptr*) );

  for(r = 0; r < rows; ++r) {
    mat[r] = (SomeStruct_ptr*) malloc(sizeof(SomeStruct_ptr) * cols);
    for(c = 0; c < cols; ++c) {
      mat[r][c] = somestruct_new(0,0);
    }
  }
  return mat;
}

SomeStructMat_ptr SomeStructMat_new(int rows, int cols) {
  SomeStructMat_ptr mat = (SomeStructMat_ptr) malloc(sizeof(SomeStructMat));
  mat->rows = rows;
  mat->cols = cols;
  mat->data = somestructmat_new(rows, cols); 
  return mat;
}

void SomeStructMat_destroy(SomeStructMat_ptr mat) {
  int r,c;
  for(r = 0; r < mat->rows; r++) {
    for(c = 0; c < mat->cols; c++) {
      free(mat->data[r][c]);
    }
    free(mat->data[r]);
  }
  free(mat->data);
  free(mat);
}

int SomeStructMat_overwrite(SomeStructMat_ptr ssm, int r, int c, SomeStruct_ptr ssp) {
  // TODO: Check that r and c are within bounds!
  somestruct_destroy(ssm->data[r][c]);
  ssm->data[r][c] = ssp;
}

void SomeStructMat_print(SomeStructMat_ptr mat) {
  int r,c;
  for(r = 0; r < mat->rows; r++) {
    for(c = 0; c < mat->cols; c++) {
      printf("[%d, %d] ", mat->data[r][c]->foo, mat->data[r][c]->bar);
    }
    printf("\n");
  }
}

int main(int argc, char** argv) {
  SomeStructMat_ptr ssm = SomeStructMat_new(2,3);

  SomeStructMat_print(ssm);

  printf("----------------\n");

  SomeStructMat_overwrite(ssm, 0, 1, somestruct_new(5,6));

  SomeStructMat_print(ssm);

  printf("----------------\n");

  SomeStructMat_destroy(ssm);

  return 0;
}

please don't use pointer typedefs. You've malloc'd the wrong number of bytes in the first malloc, `somestructmat_new` dereferences uninitialized pointers, and the second malloc doesn't even compile — M.M, Feb 17 '15 at 01:11
@nullgraph it makes the code harder to read. When looking at code with pointers you want to build up a mental image of what allocations are where and what's pointing at what. Having the `*` symbols right there helps with that , but when they are hidden it is harder to see what is going on. — M.M, Feb 17 '15 at 01:23
The only time you'd actually want to do it would be for an opaque type that happens to be implemented as a pointer, but that isn't the situation here. — M.M, Feb 17 '15 at 01:24
@MattMcNabb is absolutely right, this is sloppy code. It does however compile and run, you can certainly take it from there. Indeed looking at a large code base, trying to figure out whether or not something is a pointer, is a bad experience. In most cases it is best to avoid these types of typedefs. — user1732602, Feb 17 '15 at 01:29
@MattMcNabb I tried the code in the answer. It didn't compile at first, then I realized I was using g++. It however compiles and runs under gcc. — nullgraph, Feb 17 '15 at 02:25

Multidimensional array element assignment with pointer

3 Answers3