0

I'm trying to make a struct that generates a random matrix and am getting "error: expected â=â, â,â, â;â, âasmâ or â_attribute_â before âmatrixâ" when compiling. How can I get this to work effectively and efficiently?

I guess expected errors usually are caused by typos but I don't see any.

I'm very new to C so pointers and malloc are quite foreign to me. I really appreciate your help.

/* It's called RandomMatrixMaker.c */

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

typdef struct {
  char* name;
  int MID;
  int MRows;
  int MCols;
  long[][]* MSpace;
} matrix;

matrix makeRIDMatrix(char* name, int MID, int MRows, int MCols) {
  matrix m;
  static int i, j, r;
  m.name = name;
  m.MID = MID;
  m.MRows = MRows;
  m.MCols = MCols;
  for (i=0; i<m.MRows; i++) {
    for (j=0; i<m.MCols; j++) {
      r = random(101);
      *(m.MSpace[i][j]) = r; 
    }
  }
  return m;
}

int main(void) {
  makeRIDMatrix("test", 1, 10, 10);
  return 0;
}
user438383
  • 5,716
  • 8
  • 28
  • 43
PrckPgn
  • 37
  • 2
  • 6

3 Answers3

4

There is indeed a typo. You misspelled typedef:

typdef struct {

should be:

typedef struct {

EDIT:

Also, there's no reason to use static here:

static int i, j, r;

You can just get rid of the static modifier.

int i, j, r;
Mysticial
  • 464,885
  • 45
  • 335
  • 332
  • Thanks for that, and now, of course, I have other issues. How can I have a variable sized array in a struct that is defined by the # rows and # cols during creation? – PrckPgn Oct 30 '11 at 23:27
  • 3
    That should probably be asked as a separate question. I'm not too familiar with variable sized arrays in struct myself. But you can try looking at these questions: (http://stackoverflow.com/questions/688471/variable-sized-struct-c), (http://stackoverflow.com/questions/1223876/how-to-create-a-structure-with-two-variable-sized-arrays-in-c). – Mysticial Oct 30 '11 at 23:31
2

As another poster mentioned, there's a typo, but even with that corrected, it wouldn't compile, due to the definition of matrix.MSpace.

Let's begin in makeRIDMatrix(). You've declared an automatic (stack) variable of type "matrix". At the end of the function, you return that object. Whilst this is permissible, it's not advisable. If the struct is large, you will be copying a lot of data unnecessarily. Better to pass a pointer to a matrix into makeRIDMatrix(), and have makeRIDMatrix() fill in the contents.

The test in the inner loop is against i, but should be against j.

Next, let's look at the definition of "matrix". The definition of "MSpace" is a mess, and wouldn't even compile. Even if it did, because you haven't defined the length of a row, the compiler would not be able to calcuate the offset to any given item in the array. You want a two-dimensional array without giving the row length, but you can't do that in C. You can in other languages, but not C.

There's a lot more I could point out, but I'd be missing the real point. The real point is this:

C Is Not Java.

(It's also not one of the interpreted languages such as JavaScript, PHP, Python, Ruby and so on.)

You don't get dynamically-expanding arrays; you don't get automatic allocation of memory; you don't get garbage collection of unreferenced memory.

What you need is something more like this:

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

typedef struct {
  char* name;
  int MID;
  unsigned int MRows;
  unsigned int MCols;
  long *MSpace;
} matrix;

void makeRIDMatrix(matrix *pmx, char* name, int MID,
                   unsigned int MRows, unsigned int MCols) {
  int i, j;
  long *MSpace = malloc(sizeof(*MSpace)*MRows*MCols);
  if (MSpace == NULL) {
    return;
  }
  pmx->name = name;
  pmx->MID = MID;
  pmx->MRows = MRows;
  pmx->MCols = MCols;
  pmx->MSpace = MSpace;

  srandom((unsigned int)time(NULL));
  for (i=0; i<MRows; i++) {
    for (j=0; i<MCols; j++) {
      long int r = random() % 101L;
      *(MSpace++) = r;
    }
  }
}

inline long * item_addr(const matrix *pmx, 
                        unsigned int row, unsigned int col) {
  if (pmx == NULL || pmx->MSpace == NULL
      || row >= pmx->MRows || col >= pmx->MCols) {
    return NULL;
  }
  return &(pmx->MSpace[row * pmx->MCols + col]);
}

long get_item(const matrix *pmx, unsigned int row, unsigned int col) {
  long *addr = item_addr(pmx, row, col);
  return addr == NULL ? 0L : *addr;
}

void set_item(matrix *pmx, 
              unsigned int row, unsigned int col, 
              long val) {
  long *addr = item_addr(pmx, row, col);
  if (addr != NULL) {
    *addr = val;
  }
}

int main(void) {
  matrix m;
  makeRIDMatrix(&m, "test", 1, 10, 10);
  return 0;
}

Note a few things here. Firstly, for efficiency, I fill the array as if it were one-dimensional. All subsequent get/set of array items should be done through the getter/setter functions, for safety.

Secondly, a hidden nasty: makeRIDMatrix() has used malloc() to allocate the memory - but it's going to be job of the calling function (or its successors) explciitly to free() the allocated pointer when it's finished with.

Thirdly, I've changed the rows/cols variables to unsigned int - there's little sense in definining an array with negative indices!

Fourthly: little error checking. For example, makeRIDMatrix() neither knows nor cares whether the parameter values are sensible (e.g. the matrix pointer isn't checked for NULLness). That's an exercise for the student.

Fifthly, I've fixed your random number usage - after a fashion. Another exercise for the student: why is the way I did it not good practice?

However - all of this is moot. You need to get yourself a good C textbook, or a good online course, and work through the examples. The code you've given here shows that you're punching above your weight at the moment, and you need to develop some more C muscles before going into that ring!

JonGreen
  • 369
  • 1
  • 5
  • Thank you very much for your thoughtful response. I'm taking an embedded programming course at the moment and am able to perform well with detailed apis, but on my own I feel 4 inches tall. Will take your advice to heart. – PrckPgn Oct 31 '11 at 01:25
1

In relation to your question about "variable sized arrays", you could have something like:

/* can stick this into your struct, this is just an example */
size_t rows, cols;
long **matrix;

/* set the values of rows, cols */

/* create the "array" of rows (array of pointers to longs) */
matrix = (long**)malloc(rows * sizeof(long*));

/* create the array of columns (array of longs at each row) */
for (i = 0; i < rows; i++)
   matrix[i] = (long*)malloc(cols * sizeof(long));

/* ... */

/* free the memory at the end */
for (i = 0; i < rows; i++)
   free(matrix[i]);

free(matrix);

Then you can just access the dynamically allocated matrix similar to any other array of arrays.

ie. to set element at the first row (row 0) and fourth column (column 3) to 5:

matrix[0][3] = 5;
AusCBloke
  • 18,014
  • 6
  • 40
  • 44