0 
typedef struct {
    int num_rows;
    int num_cols;
    int** data;
} BinaryMatrix;


BinaryMatrix *ConstructBinaryMatrix(int num_rows, int num_cols) {
    BinaryMatrix matrix = {
            .num_rows = num_rows,
            .num_cols = num_cols,
            .data = (int **) malloc((num_rows) * sizeof(int *)),
    };

    int i;
    for (i = 0; i < num_cols; i++) {
        matrix.data[i] = (int *) malloc(num_cols * sizeof(int));
    }
    return &matrix;

}

Is this the correct way to define a BinaryMatrix, and how to initialize it?

Thanks for your help.

I got the following error.

BinaryMatrix* M;
M = ConstructBinaryMatrix(2, 2);
printf("%d:%d", M->num_rows, M->num_cols);

The output is: 4198012:0

Improve this question
21
  • 1
    int** data is not a matrix (aka 2D array). And it cannot point to such a type. Commented Mar 7, 2016 at 23:46
  • 1
    @Olaf: int ** is of course the standard way of simulating a dynamically-allocated two-dimensional array in C, so I'm not sure why you are insisting it is "not a matrix". Commented Mar 7, 2016 at 23:59
  • 1
    If you use int** you break the contiguous storage rule and make it hard to connect with other APIs that assumes this rule. Also it forces the compiler to assumes pointers to each row aliases which is an unnecessary performance penalty. The continence of [][] pretty means nothing to a experienced C programmer. Commented Mar 8, 2016 at 0:06
  • 1
    @SteveSummit: int ** cannot even represent a 2D array. A pointer is not an array. You cannot allocate or free it with a single malloc, etc. It is just used that often because people cannot handle tghe pointer and array syntax of C correctly and because of bad advice by so-called experts. int (*array)[DIM] is the correct syntax for a 2D array (resp. a pointer to a 1D array). Commented Mar 8, 2016 at 0:15
  • 1
    @SteveSummit As Olaf already said, an int ** is no 2D array but an array of pointers (to possibly arrays). For the syntax: One shouldn't expose (or force) direct access to the data member, but rather provide a meaningful int element(BinaryMatrix const * m, size_t column, size_t row) to decouple user of the matrix and it's implementation / representation. Commented Mar 8, 2016 at 0:45

3 Answers 3

Reset to default
2

You are returning a pointer to local data, which is bad form and doesn't work.

You should probably call malloc to allocate space for a BinaryMatrix structure, something like this:

BinaryMatrix *ConstructBinaryMatrix(int num_rows, int num_cols) {
    BinaryMatrix *matrix = malloc(sizeof(BinaryMatrix));
    matrix->num_rows = num_rows;
    matrix->num_cols = num_cols,
    matrix->data = malloc(num_rows * sizeof(int *));

    int i;
    for (i = 0; i < num_rows; i++) {
        matrix->data[i] = malloc(num_cols * sizeof(int));
    }
    return matrix;
}

(Also I have fixed the loop bounds, as M.M. pointed out.)

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

"Doesn't work" doesn't adequately describe the situation. With undefined behavior, it may indeed appear to work perfectly.
1

i < num_cols should be i < num_rows.

Currently your code returns the address of a local variable. Local variables are destroyed when the function returns, so you in fact return a dangling pointer, not a good idea.

Instead you should return a copy of the local variable. Remove the * from the function prototype and from declaration of M, and remove the & from the return statement.

Improve this answer

2 Comments

Thanks,function prototype cannot be changed.
@KKOCA you could go into your editor, move the cursor over to the prototype, and press some keys. If you don't want to change it then you should update your question to include this restriction
-2

Unless its very large I would use:

#define M 3
#define N 3

int matrix[M][N]={{0,0,0},{0,0,0},{0,0,0}};
/* above possibly static */

Simples !

If the matrix was large simply use for loops to initialize.

Improve this answer

9 Comments

This is clearly against the intent of OP's code, where the number of rows and columns seems to be unknown at compile time.
@DanielJour replace #define by variables then. In fact OP's example uses literals 2, 2.
@M.M That won't work, at least not in standard C. Providing a functional abstraction for matrices of different sizes is much better than hard coding a particular size.
@M.M: A static array cannot have variable size.
@Olaf The array should be non-static
|