initialize struct of C

Question

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

Possible duplicate of [How to initialize a struct in ANSI C](http://stackoverflow.com/questions/330793/how-to-initialize-a-struct-in-ansi-c) — manniL, Mar 07 '16 at 23:39
BinaryMatrix* M; M = ConstructBinaryMatrix(2, 2); printf("%d:%d", M->num_rows, M->num_cols); I got: 4198012:0 — , Mar 07 '16 at 23:42
Try `BinaryMatrix* M = { .num_rows = 2, .num_cols = 2 };` I don't think you're allowed to assign values to a struct using parentheses. — Chirality, Mar 07 '16 at 23:45
`int** data` is not a matrix (aka 2D array). And it cannot point to such a type. — too honest for this site, Mar 07 '16 at 23:46
You must not return a pointer to an object of automatic duration (as you do), because such an object's lifetime ends the moment the function returns. Dereferencing such a pointer later produces undefined behavior. — John Bollinger, Mar 07 '16 at 23:47
The function structure is fixed , I have to return a pointer. Please tell me how to do. — , Mar 07 '16 at 23:50
`int * data` as well as `malloc(sizeof(int) * rows * columns)` would be a much better idea for a matrix. (Note: Do not cast the return value of `malloc`) — Daniel Jour, Mar 07 '16 at 23:50
@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". — Steve Summit, Mar 07 '16 at 23:59
@DanielJour: the "flattened" 2D array you're allocating with `malloc(sizeof(int) * rows * columns)` is hard to use, because you cannot use `[][]` to access the individual cells. — Steve Summit, Mar 08 '16 at 00:00
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. — user3528438, Mar 08 '16 at 00:06
@ KKOCA If the post by @ SteveSummit solved the problem, please click the check mark next to his post. — user3386109, Mar 08 '16 at 00:11
@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). — too honest for this site, Mar 08 '16 at 00:15
@Olaf stop trying to enforce your personal preference on everyone else — M.M, Mar 08 '16 at 00:18
@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. — Daniel Jour, Mar 08 '16 at 00:45
Guys: I think I know the difference between an `int **` and an `int [][]`, and the difficulties inherent in interfacing between the two. (See http://c-faq.com/aryptr/ary2dfunc3.html .) What I said was that `int **` is a standard way of **simulating** a 2D array -- and it's clearly the way that the OP was asked to do it. — Steve Summit, Mar 08 '16 at 03:38
@SteveSummit what I was all about is that using an `int *` here is the better choice (OP seems to be only constrained with regard to the function signature). While using `int **` seems to be the "standard way of simulating" 2D arrays, it's in fact more of an anti pattern: just quickly scroll over the C tagged posts about arrays (or even worse, matrices) of the last days. It's a constant source of confusion (this is also based on personal teaching experience). That said, there are of course situations calling for the use of this double indirection (swapping rows comes to my mind). — Daniel Jour, Mar 08 '16 at 08:45
What @Olaf is probably concerned with is the misuse of terminology. `int **` just is **no** 2d array. Calling it for what it is also helps (less experienced) people with understanding the whole concept behind pointers and arrays. — Daniel Jour, Mar 08 '16 at 08:46
@DanielJour: Exactly. I don't mind much if someone uses an `int **`, but it is plain wrong calling that "2D array" or "matrix". Array semantics are just broken in C and results in massive confusion for newbies. We should use the correct terms and make clear what's going on not to fortify these wrong impressions. In general using a true 2D array **is** the better approach considering all implications (fragmentation, locality, handling, etc.) For the question above, I'd have to think a bit how to solve the issue and if an array of pointers .. might be the way to go, though. — too honest for this site, Mar 08 '16 at 13:59

Steve Summit · Accepted Answer · 2016-03-07T23:54:47.200

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.)

edited Mar 07 '16 at 23:54

answered Mar 07 '16 at 23:50

Steve Summit

45,437
7
70
103

2

"Doesn't work" doesn't adequately describe the situation. With undefined behavior, it may indeed appear to work perfectly. – Fiddling Bits Mar 08 '16 at 00:47

score 1 · Answer 2 · answered Mar 07 '16 at 23:48

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.

answered Mar 07 '16 at 23:48

M.M

138,810
21
208
365

Thanks,function prototype cannot be changed. – Mar 07 '16 at 23:53
@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 – M.M Mar 07 '16 at 23:57

score -2 · Answer 3 · answered Mar 07 '16 at 23:50

-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.

answered Mar 07 '16 at 23:50

Arif Burhan

507
4
12

This is clearly against the intent of OP's code, where the number of rows and columns seems to be unknown at compile time. – Daniel Jour Mar 07 '16 at 23:51
@DanielJour replace `#define` by variables then. In fact OP's example uses literals `2, 2`. – M.M Mar 08 '16 at 00:02
@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. – Daniel Jour Mar 08 '16 at 00:49
@M.M: A static array cannot have variable size. – too honest for this site Mar 08 '16 at 00:53
@Olaf The array should be non-static – M.M Mar 08 '16 at 01:10
@DanielJour actually it would work in standard C, for the last 17 years . – M.M Mar 08 '16 at 01:10
@M.M VLA are a nice thing, as long as you don't escape the scope in which they're defined. Arrays aren't first class values in C (can neither be passed to our returned from functions), but since they're able to decay to pointers passing them to functions works fine. Now consider 2D arrays. If you pass one to a function, what's the type of the resulting pointer? `int (*array)[DIM]` Now what happens when `DIM` isn't known at compile time? What should `sizeof(array[0])` return? VLAs are nice, but higher dimensional VLAs just suck. – Daniel Jour Mar 08 '16 at 01:18
@DanielJour `sizeof(array[0])` would be `sizeof(int) * DIM`. I disagree that they suck, this syntax is useful and more readable than a bunch of arithmetic. – M.M Mar 08 '16 at 01:28
Although in this answer, VLAs cannot be used with initializers so `{{0,0,0` etc. would need to be replaced by a loop or a `memset` – M.M Mar 08 '16 at 01:29

initialize struct of C

3 Answers3