Using Malloc to add two arrays HW in C

Question

This program is supposed to ask the user to enter two matrices and provide the sum of the two. When compiled it does not work as expected, I believe it is due to my use of malloc, if anyone can help this would be greatly appreciated.

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


/*Here we declare and define our function 'matrices', which prompts the
 user for Matrix A and Matrix B and stores their values.*/

int matrices(int rows, int cols){

    int i;

    int** matrixA = malloc(rows * sizeof(**matrixA));

    int** matrixB = malloc(rows * sizeof(**matrixB));

    printf("Enter Matrix A\n");

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

         scanf("%d", matrixA[i]);

    }

    printf("Enter Matrix B\n");

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

         scanf("%d", matrixB[i]);

    }

    return 0;
}

/*Here we declare and define our function 'sum', which sums Matrix A and 
Matrix B and provides us with the summation of the two in the 
variable 'matrixSum'*/

int sum(int rows, int cols){

    int i;

    int** matrixA = malloc(rows * sizeof(**matrixA));

    int** matrixB = malloc(rows * sizeof(**matrixB));

    int** matrixSum = malloc(rows * sizeof(**matrixSum));



    printf("A + B =\n");

    for (i = 0; i < rows; i++) {

        matrixA[i] = (int *) malloc(cols * sizeof(int));
        matrixB[i] = (int *) malloc(cols * sizeof(int));
        matrixSum[i] = (int *) malloc(cols * sizeof(int));

           *matrixSum[i] = *matrixA[i] + *matrixB[i];
        printf("%d\t", *matrixSum[i]);


            printf("\n");
    }

    return 0;
}
/*Here we declare and define our main function which works to prompt the user for the number of rows and columns and calls the previously declared functions. */

int main(void){

    int rows, cols;

    int** matrixA = malloc(rows * sizeof(**matrixA));

    int** matrixB = malloc(rows * sizeof(**matrixB));

    int** matrixSum = malloc(rows * sizeof(**matrixSum));

    printf("Please enter the number of rows: ");
    scanf("%d", &rows);
    printf("Please enter the number of columns: ");
    scanf("%d", &cols);


    matrices(rows, cols);
    sum(rows, cols);

    free(matrixA);
    free(matrixB);
    free(matrixSum);

    return 0;
}

I guess `int** matrixA = malloc(rows * sizeof(**matrixA));` abd other lines like this should be like `int** matrixA = malloc(rows * sizeof(*matrixA));`. Also casting the result of `malloc()` explicitly is considered as [not good](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). — MikeCAT, May 15 '16 at 05:49
`scanf("%d", matrixA[i]);` is OK, but looks weird. It will read data only to the first element of the array. — MikeCAT, May 15 '16 at 05:51
Using values in buffer allocated via `malloc()` and not initialized invokes *undefined behavior*. — MikeCAT, May 15 '16 at 05:54
where exactly do I do that though? I'm not sure what you are referring to, malloc is a new concept to me — Nick Capranica, May 15 '16 at 05:56
consider using a single allocation for each matrix, it will simplify your code — M.M, May 15 '16 at 06:34

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

The size of what is pointed by int** matrixA; will be sizeof(*matrixA), not sizeof(**matrixA). In environments in which size of pointers and int differs, using latter will make the buffer too small.
In the function matrices(), some input are read and then discarded. Memory leak also happens.
In function sum(), indeterminate values in buffer allocated via malloc() and not initialized are used and undefined behavior is invoked.
They say you shouldn't cast the result of malloc() in C.

To correct:

Do proper size calculation.
Avoid causing memory leak, keep and pass what is read.
Use loops to read all elements, not only the first elements of each row.

Here is a fixed code with error checking:

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

void matrices(int rows, int cols, int*** matrixA, int*** matrixB){

    int i, j;

    *matrixA = malloc(rows * sizeof(**matrixA));
    if (*matrixA == NULL){
        perror("matrixA malloc");
        exit(1);
    }

    *matrixB = malloc(rows * sizeof(**matrixB));
    if (*matrixB == NULL){
        perror("matrixB malloc");
        exit(1);
    }

    printf("Enter Matrix A\n");

    for (i = 0; i < rows; i++){
        (*matrixA)[i] = malloc(cols * sizeof(int));
        if ((*matrixA)[i] == NULL){
            perror("matrixA[i] malloc");
            exit(1);
        }

        for (j = 0; j < cols; j++){
            if (scanf("%d", &(*matrixA)[i][j]) != 1){
                fputs("matrixA read error\n", stderr);
                exit(1);
            }
        }

    }

    printf("Enter Matrix B\n");

    for (i = 0; i < rows; i++){
        (*matrixB)[i] =  malloc(cols * sizeof(int));
        if ((*matrixB)[i] == NULL){
            perror("matrixB[i] malloc");
            exit(1);
        }

        for (j = 0; j < cols; j++){
            if (scanf("%d", &(*matrixB)[i][j]) != 1){
                fputs("matrixB read error\n", stderr);
                exit(1);
            }
        }

    }

}

void sum(int rows, int cols, int** matrixA, int** matrixB, int*** matrixSum){

    int i, j;

    *matrixSum = malloc(rows * sizeof(**matrixSum));
    if (*matrixSum == NULL){
        perror("matrixSum malloc");
        exit(1);
    }

    printf("A + B =\n");

    for (i = 0; i < rows; i++) {

        (*matrixSum)[i] = malloc(cols * sizeof(int));
        if ((*matrixSum)[i] == NULL){
            perror("matrixSum[i] malloc");
            exit(1);
        }

       for (j = 0; j < cols; j++){
           (*matrixSum)[i][j] = matrixA[i][j] + matrixB[i][j];
           printf("%d\t", (*matrixSum)[i][j]);
       }

        printf("\n");
    }

}
/*Here we declare and define our main function which works to prompt the user for the number of rows and columns and calls the previously declared functions. */

int main(void){

    int rows, cols;

    int i;

    int** matrixA;
    int** matrixB;
    int** matrixSum;

    printf("Please enter the number of rows: ");
    if (scanf("%d", &rows) != 1){
        fputs("rows read error\n", stderr);
        return 1;
    }
    printf("Please enter the number of columns: ");
    if (scanf("%d", &cols) != 1){
        fputs("cols read error\n", stderr);
        return 1;
    }

    matrices(rows, cols, &matrixA, &matrixB);
    sum(rows, cols, matrixA, matrixB, &matrixSum);

    for (i = 0; i < rows; i++) {
        free(matrixA[i]);
        free(matrixB[i]);
        free(matrixSum[i]);
    }

    free(matrixA);
    free(matrixB);
    free(matrixSum);

    return 0;
}

Note that what is pointed by *matrixA is **matrixA, so using sizeof(**matrixA) is now correct.

wow that was awesome thank you. Just another quick question, I am not allowed to have a for loop in main so how might I deallocate space? Would I create another function just for that purpose or do it in the other functions I previously had? — Nick Capranica, May 15 '16 at 06:44
Tried creating a function and did not work out so well. I tried creating a void function w arguments rows, matrixA, matrixB, and matrixSum and including the for loop to deallocate space but it is not working as I had thought.... — Nick Capranica, May 15 '16 at 06:51
Just got it to work, thanks for all your help man appreciate it — Nick Capranica, May 15 '16 at 07:05

xaxxon · Answer 2 · 2016-05-15T06:16:28.437

0

    matrixA[i] = (int *) malloc(cols * sizeof(int));

Shouldn't that be sizeof(int*)? or

int** matrixA

This be a single pointer?

If you want to dynamically allocate a 2d array of ints, you start with an int **, allocate a bunch of int*'s for it, then, for each of the int *'s, you allocate a bunch of ints.

int ** array2d = malloc(rows * sizeof(int*));
for(int i = 0; i < rows; i++) {
    array2d[i] = malloc(columns * sizeof(int));
    for (int j = 0; j < columns; j++) {
        scanf("%d\n", &array2d[i][j]);
    }
}

edited May 15 '16 at 06:16

answered May 15 '16 at 05:43

xaxxon

19,189
5
50
80

haha I'm not entirely sure, I actually meant to post this as a question as the program was not performing properly – Nick Capranica May 15 '16 at 05:46
I guess it shouldn't because `matrixA[i]` is type `int*` and pointer to (the first element of) array of `int` should be assigned to it. – MikeCAT May 15 '16 at 05:46
the problem w my code is that when compiled it does not let me enter the ample amount of columns and rows and returns 0's – Nick Capranica May 15 '16 at 05:49
And you don't think some really bad memory management could cause that? – xaxxon May 15 '16 at 06:06
lol I realize that can definitely be the problem but my assignment was to use malloc instead of 2 dimensional arrays w pre-defined values, could you elaborate on how I might fix my memory management? I know somewhere in the sum it's messed up – Nick Capranica May 15 '16 at 06:09
updated answer. -- re-updated the answer with the scanf loop, too – xaxxon May 15 '16 at 06:16

Rishikesh Raje · Answer 3 · 2016-05-15T06:17:44.733

0

The problem is in the for loop. The malloc inside the loop looks OK, however, you need another nested loop to scan the all the rows and columns of the matrix.

for (i = 0; i < rows; i++){
    matrixA[i] = (int *) malloc(cols * sizeof(int));
    for (j=0; j<cols; j++)
    {
       scanf("%d", &matrixA[i][j]);
    }
}

edited May 15 '16 at 06:17

answered May 15 '16 at 06:10

Rishikesh Raje

8,556
2
16
31

`scanf("%d", matrixA[i][j]);` -> *undefined behavior*: passing data having wrong type and using value in buffer allocated via `malloc()` and not initialized. – MikeCAT May 15 '16 at 06:10
and it's allocating an int, not an int *, so if int is smaller, you're writing out of bounds – xaxxon May 15 '16 at 06:11
@xaxxon as per my understanding it should allocate sizeof(int) for matrixA[i]. Please explain your point of view – Rishikesh Raje May 15 '16 at 06:23
@MikeCAT fixed to `&matrixA[i][j]` – Rishikesh Raje May 15 '16 at 06:24
@RishikeshRaje if matrixA is an int**, then allocating ints to it is nonsensical – xaxxon May 15 '16 at 07:04
@xaxxon Allocating to `matrixA[i]` is not allocating to `matrixA`. – MikeCAT May 15 '16 at 07:12
oh, i think I was still thinking about the old version: scanf("%d", matrixA[i][j] of the answer. – xaxxon May 15 '16 at 07:20

Using Malloc to add two arrays HW in C

3 Answers3