segmentation fault on simple loop

Question

I'm trying to write a program that calculates the number of elements of given matrix,for example M[2][3] = [1,2,3][3,4,5] should give the number of elements from 0 trough 9 in a nxm array. The algorithm is correct up until line 34(M[i][j] = i + 2;) in which I'm having a segmentation fault. What am I doing wrong?

#include<stdio.h>
#include<stdlib.h>
#define ROW 10
#define COL 10
#define MAX 10
void Print_Matrix(int **M,int row,int col);
int MallocX(int **M,int row,int col);
int main(void)
{
    int **M = 0x0;
    int count[MAX] = {0};
    int i,j;
    
    
    
    if(MallocX(M,ROW,COL)){
        fprintf(stdout,"Could not allocate memory\n");
        exit(1);
    }

    for(i = 0;i<ROW;i++){
        for(j = 0;j<COL;j++){
            M[i][j] = i + 2;
        }
    }
    Print_Matrix(M,ROW,COL);
    for(i = 0;i<ROW;i++){
        for(j = 0;j<COL;j++){
            ++count[M[i][j]];
        }
    }
    
    for(j = 0;j<MAX;j++){
        if(count[j]){
            printf("%d %d\n",j,count[j]);
        }
    }
    for(i = 0;i<ROW;i++){
        free(M[i]);
    }
    free(M);
}
int MallocX(int **M,int row,int col)
{
    int i;
    M = (int **) malloc(row * sizeof(int *));
    if(M == NULL){
        fprintf(stderr,"Error allocating memory\n");
        free(M);
        return 1;
    }

    for(i = 0;i<row;i++){   
        M[i] = (int *) malloc(col * sizeof(int));
        if(M[i] == NULL){
            fprintf(stderr,"Error allocating memory\n");
            free(M[i]);
            return 1;
        }
    }
    return 0;
}
void Print_Matrix(int **M,int row,int col)
{
    int i,j;
    for(i = 0;i<row;i++){
        for(j = 0;j<col;j++){
            printf("%d ",M[i][j]);
        }
        printf("\n");
    }
}

You're writing past the end of count. The maximum valid index into count is MAX-1, i.e. 9. The largest value in M is 9+2 = 11. So you're writing two elements past the end with ++count[M[i][j]]. — Tom Karzes
– Tom Karzes, Commented Jul 9, 2021 at 2:02
All arguments in C are passed by value. You can simulate pass-by-reference by explicitly passing the address of a variable, but you aren't doing that. So, that's your main bug. — Tom Karzes
– Tom Karzes, Commented Jul 9, 2021 at 2:08

SGeorgiades · Accepted Answer · 2021-07-10 18:49:12Z

3

This is because you are passing M by value, not reference. Your MallocX allocates memory for your matrix, but those allocations are orphaned when you return to the main program, where M is still 0x0 (or NULL), which is why allocation functions typically return a pointer. Perhaps you want something like this:

int **MallocX(int row,int col)
{
    int **Matrix, i;

    Matrix = malloc(row * sizeof(int*));
    if(Matrix == NULL) {
        fprintf(stderr,"Error allocating memory\n");
        return NULL;
    }

    for (i = 0; i < row; i++) {
        Matrix[i] = (int*) malloc(col * sizeof(int));
        if(Matrix[i] == NULL){
            fprintf(stderr,"Error allocating memory (%d)\n",i);
            for (int j = 0; j < i; ++j)
                free(Matrix[j]);
            free(Matrix);
            return NULL;
        }
    }
    return Matrix;
}

Then in main, call it with:

if (!(M = MallocX(ROW,COL)) {
    fprintf(stdout,"Could not allocate memory\n");
    exit(1);
}

Note that in your original code, you were calling free(M) when M was NULL, which will itself cause a segfault. So I've tidied up your garbage collection a bit too.

edited Jul 10, 2021 at 18:49

answered Jul 9, 2021 at 2:10

SGeorgiades

1,8311 gold badge12 silver badges11 bronze badges

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

2 Comments

user3386109 Over a year ago

Calling free with a NULL pointer is allowed by the C standard. So OP's free calls were useless, but also harmless.

SGeorgiades Over a year ago

I was not aware of that, @user3386109. Learn something new every day!

Collectives™ on Stack Overflow

segmentation fault on simple loop

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related