Member Avatar for Member #849548

Hi all,

I trying to add two matrices together using a typedef struct, but i seems to get an error doing this. I asked my professor, but he couldnt help me. Is there anyone who might know why this problem is caused?

Thank you so much!

Code:

/* Structures */
typedef struct matrix {
    int rows;
    int cols;
    float **mat;
} Matrix;


Matrix matrix_add(Matrix a, Matrix b){
    Matrix m;

    int i, j;
    for (i=0; i<a.rows; i++) {
        for (j=0; j<a.cols; j++) {
            m.mat[i][j] = (a.mat[i][j]+b.mat[i][j]);
        }
    }
    m.rows = a.rows;
    m.cols = a.cols;

    return m;
}
Edited by Reverend Jim because: Fixed formatting
  • 4 Contributors
  • 6 Replies
  • 3K Views
  • 5 Years Discussion Span
  • Latest Post Latest Post by Akbar_5

Dani AI

Generated

Nice catch by @Ancient Dragon: you were writing into m.mat before allocating it. Also, in your follow-up you have a small typo: m.cols = m.cols; should be m.cols = a.cols;. As hinted, keep the algorithm simple and make memory management explicit. Two practical tips: (1) verify that the input matrices have identical dimensions before adding, and (2) allocate the result first, then fill it.

Here is a compact pattern that keeps rows contiguous (better cache locality) and avoids casting malloc in C. It also returns a status so you can report errors cleanly.

#include <stdlib.h>

typedef struct {
    size_t rows, cols;
    float **row;  /* row[i][j] */
} Matrix;

static int matrix_alloc(Matrix *m, size_t r, size_t c) {
    size_t i;
    m->rows = r; m->cols = c;
    m->row = malloc(r * sizeof m->row[0]);
    if (!m->row) return -1;
    m->row[0] = malloc(r * c * sizeof m->row[0][0]);
    if (!m->row[0]) { free(m->row); return -1; }
    for (i = 1; i < r; ++i) m->row[i] = m->row[0] + i * c;
    return 0;
}

static void matrix_free(Matrix *m) {
    if (m && m->row) { free(m->row[0]); free(m->row); }
}

int matrix_add(const Matrix *a, const Matrix *b, Matrix *out) {
    size_t i, j;
    if (a->rows != b->rows || a->cols != b->cols) return -1;
    if (matrix_alloc(out, a->rows, a->cols) != 0) return -1;
    for (i = 0; i < a->rows; ++i)
        for (j = 0; j < a->cols; ++j)
            out->row[i][j] = a->row[i][j] + b->row[i][j];
    return 0;
}

Why this way? Contiguous 2D allocation simplifies freeing and is a well-known approach in C (comp.lang.c FAQ: dynamic 2D arrays). Also, in C you do not need (and generally should not) cast malloc’s return value; just include <stdlib.h> (FAQ: why not cast malloc). Finally, prefer sizeof *ptr style to keep types in sync (FAQ: multiplying by sizeof).

Recommended Answers

All 6 Replies

Member Avatar for Adak

You've got a pointer to a pointer - and you've set aside no memory for it, at all.

The algorithm may be OK, but it's not one I'd recommend you use. Strive for clarity and simplicity in your code.

adding two matrices into a third matrix which is NOT a part of a struct, is very much the way I'd suggest.

Member Avatar for Ancient Dragon

what errors are you getting?

In matrix_add() function you are trying to use an uninitialized pointer m.mat. It has to be allocated memory before setting the array elements in those loops. Move the two lines m.rows = and m.cols = up above the two loops then allocate memory for m.mat.

Member Avatar for Member #849548

what errors are you getting?

In matrix_add() function you are trying to use an uninitialized pointer m.mat. It has to be allocated memory before setting the array elements in those loops. Move the two lines m.rows = and m.cols = up above the two loops then allocate memory for m.mat.

Thats was absolutely fantastic. I allocated memory by using:

/* Allocating mem for m.mat */
mat_temp = (float**) malloc(a.rows * sizeof(float*));
for (i = 0; i < a.rows; i++){
    mat_temp[i] = (float*) malloc(a.cols * sizeof(float));
}

m.rows = a.rows;
m.cols = m.cols;
m.mat = mat_temp;

and then everything worked!

Thank you so much for you help. I can finally relax a bit now.

Edited by Reverend Jim because: Fixed formatting
Member Avatar for Member #849548

PS: how do I mark this post as "SOLVED" ?

Member Avatar for Ancient Dragon

Look at the information just above the Message edit box. It has a link to mark the thread as solved.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.