Segmentation fault (2D array with dynamic memory allocation) in C

Question

I am still getting a hang of using malloc, calloc or realloc. Pretty sure I'm getting a segmentation fault because of an incorrect pointer or something, but for the life of me, I don't understand where I'm doing it wrong. The program works when the user input is '5 5', anything other than that causes the program to act weird and often times fail with a segmentation fault. Matrix A and B should be filled with random numbers, and Matrix C should sum matrix A and B, all the while using dynamic memory allocation. I'm not familiar with allocs, I've just started to understand pointers and 2D arrays. Can somebody explain what I'm doing wrong. Here's the whole code.

#include <stdio.h>
#include <stdlib.h>
#define ROWS 10
#define COLUMNS 10

void fillMatrix (int *matrix, int rows, int columns)
{
    int i, j;
    for(i = 0; i < rows; i++)
    {
        for(j= 0; j < columns; j++)
        {
            matrix[i * COLUMNS + j] = (rand() % 10) + 1;
        }
    }
}
void printMatrix (int *matrix, int rows, int columns)
{
    int i, j;
    for(i = 0; i < rows; i++)
    {
        for(j= 0; j < columns; j++)
        {
            printf("%2d ", *(matrix + i * COLUMNS + j));
        }
    printf("\n\n");
    }
}
void sumMatrix (int *matrix, int *matrixA, int *matrixB, int rows, int columns)
{
    int i, j;
    for(i = 0; i < rows; i++)
    {
        for(j= 0; j < columns; j++)
        {
            matrix[i * COLUMNS + j] = matrixA[i * COLUMNS + j] + matrixB[i * COLUMNS + j];
        }
    }
}

int main()
{
    int *matrixA = NULL, *matrixB = NULL, *matrixC = NULL;
    int matrix[ROWS][COLUMNS], r, s, i;
    srand((unsigned) time(NULL));
    do{
    printf("Rows and columns (min 1 1, max 10 10): ");
    scanf("%d %d", &r, &s);
    }while(r > ROWS || r < 1 || s > COLUMNS || s < 1);

    matrixA = malloc(r * sizeof(int *));
    for (i = 0; i < r; i++)
        matrixA[i] = malloc(s * sizeof(int));

    printf("\nMatrix A:\n\n");
    fillMatrix(matrixA, r, s);
    printMatrix(matrixA, r, s);

    matrixB = calloc(r, sizeof(int *));
    for (i = 0; i < r; i++)
        matrixB[i] = calloc(s, sizeof(int));

    printf("\nMatrix B:\n\n");
    fillMatrix(matrixB, r, s);
    printMatrix(matrixB, r, s);

    matrixC = calloc(r, sizeof(int *));
    for (i = 0; i < r; i++)
        matrixC[i] = calloc(s, sizeof(int));

    printf("\nSummed up (Matrix C):\n\n");
    sumMatrix(matrixC, matrixA, matrixB, r, s);
    printMatrix(matrixC, r, s);

    free(matrixA);
    free(matrixB);
    free(matrixC);
    return 0;
}

I think matrixA, B and C be of type int** (int **matrixA = NULL, **matrixB = NULL, **matrixC = NULL;) — Bruno Ferreira
– Bruno Ferreira, Commented Apr 29, 2014 at 17:51

barak manos · Accepted Answer · 2014-04-29 18:07:59Z

2

You access all your matrices as single-dimension arrays, and should therefore allocate them accordingly.

For example: int* matrixA = malloc(r*s*sizeof(int));.

That's all; no need to do the for (i = 0; i < r; i++) loops.

In addition, you are using the COLUMNS macro instead of the columns argument in all your functions.

Obviously, if COLUMNS > columns, then you'll end up with a memory access violation at some point...

edited Apr 29, 2014 at 18:07

answered Apr 29, 2014 at 17:56

barak manos

30.2k10 gold badges67 silver badges117 bronze badges

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

7 Comments

VengeanceIsMine Over a year ago

Still getting the same error. Can you test the code on your machine and make the input '3 3' for example? It crashes... Albeit, I've tried this already, the above written code is the result of my fiddling around with malloc and calloc. The problem still persists. P.S. Changing my code to this actually makes it crash on '5 5' as well now.

barak manos Over a year ago

@VengeanceIsMine: Did you get rid of the for (i = 0; i < r; i++) loops?

VengeanceIsMine Over a year ago

Yes, for example: int* matrixA = malloc(r * s * sizeof(int)); printf("\nMatrix A:\n\n"); fillMatrix(matrixA, r, s); printMatrix(matrixA, r, s);

barak manos Over a year ago

@VengeanceIsMine: You are using the preprocessor definition COLUMNS instead of the argument columns in all your functions!!! You need to change that (added to the answer above).

VengeanceIsMine Over a year ago

Okay, tried it, still not working for me. Must've misunderstood you, but the problem persisted. Used the code by BLUEPIXY, now it works. Thank you anyways!

|

user3321437 · Accepted Answer · 2014-04-29 18:25:43Z

1

The problem here is that the memory locations in your matrix are not contiguous. But in all your print/fill/sum matrix functions, you are assuming that they are.

Assume a 2x2 matrix A. Your code will allocate memory in the following way. addr1 is starting address of one array, addr2 is of the second.

A[0] addr1 (addr1+sizeof(int))
A[1] addr2 (addr2+sizeof(int))

But this is what you need:

A[0] addr1 addr1 + sizeof(int)
A[1] (addr1 + 2*sizeof(int)) (addr1 + 3*sizeof(int))

So you need to replace your malloc statements into one single malloc: matrixA = malloc(r*s*sizeof(int)) Also, you are assuming that each row has COLUMN number of elements and not 's' number. If that's the case, you should malloc: matrixA = malloc(r*COLUMNS*sizeof(int)), but I am not sure why you need the other unused columns.

Also, int matrix[ROWS][COLUMNS] seems to be unused.

edited Apr 29, 2014 at 18:25

answered Apr 29, 2014 at 18:19

user3321437

955 bronze badges

1 Comment

VengeanceIsMine Over a year ago

Oh, I understand now. Thank you for painting a picture for me! And that argument at the end is totally valid, I forgot about deleting that line. Again, thanks very much!

Iłya Bursov · Accepted Answer · 2014-04-29 17:55:02Z

0

have problem with malloc, you're trying to create array of arrays instead of 2d array, you should fix it so:

matrixA = malloc(r * c * sizeof(int)); // no any kind of additional for loop here, 1 malloc per matrix

answered Apr 29, 2014 at 17:55

Iłya Bursov

24.4k4 gold badges36 silver badges68 bronze badges

Comments

BLUEPIXY · Accepted Answer · 2014-04-29 18:02:33Z

0

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

#define ROWS 10
#define COLUMNS 10

void fillMatrix (int *matrix, int rows, int columns)
{
    int i, j;
    for(i = 0; i < rows; i++)
    {
        for(j= 0; j < columns; j++)
        {
            matrix[i * COLUMNS + j] = (rand() % 10) + 1;
        }
    }
}
void printMatrix (int *matrix, int rows, int columns)
{
    int i, j;
    for(i = 0; i < rows; i++)
    {
        for(j= 0; j < columns; j++)
        {
            printf("%2d ", matrix[ i * COLUMNS + j]);
        }
        printf("\n\n");
    }
}
void sumMatrix (int *matrix, int *matrixA, int *matrixB, int rows, int columns)
{
    int i, j;
    for(i = 0; i < rows; i++)
    {
        for(j= 0; j < columns; j++)
        {
            matrix[i * COLUMNS + j] = matrixA[i * COLUMNS + j] + matrixB[i * COLUMNS + j];
        }
    }
}

int main()
{
    int *matrixA = NULL, *matrixB = NULL, *matrixC = NULL;
    int matrix[ROWS][COLUMNS], r, s, i;
    srand((unsigned) time(NULL));
    do{
    printf("Rows and columns (min 1 1, max 10 10): ");
    scanf("%d %d", &r, &s);
    }while(r > ROWS || r < 1 || s > COLUMNS || s < 1);

    matrixA = malloc(COLUMNS*r * sizeof(int));

    printf("\nMatrix A:\n\n");
    fillMatrix(matrixA, r, s);
    printMatrix(matrixA, r, s);

    matrixB = calloc(r*COLUMNS,sizeof(int));

    printf("\nMatrix B:\n\n");
    fillMatrix(matrixB, r, s);
    printMatrix(matrixB, r, s);

    matrixC = calloc(r*COLUMNS, sizeof(int));

    printf("\nSummed up (Matrix C):\n\n");
    sumMatrix(matrixC, matrixA, matrixB, r, s);
    printMatrix(matrixC, r, s);

    free(matrixA);
    free(matrixB);
    free(matrixC);
    return 0;
}

answered Apr 29, 2014 at 18:02

BLUEPIXY

40.2k7 gold badges37 silver badges73 bronze badges

4 Comments

VengeanceIsMine Over a year ago

This one works for me. Can you just, in layman's terms, explain to me what and why should've been replaced.

BLUEPIXY Over a year ago

@VengeanceIsMine You're mixed up in the handling of the array. My answer was unified to be treated as a one-dimensional array.

BLUEPIXY Over a year ago

@VengeanceIsMine , I think it is a good COLUMNS replaced by s and columns because it is a waste of memory.

VengeanceIsMine Over a year ago

Exactly, I changed it because of this. Thanks for your input.

Collectives™ on Stack Overflow

Segmentation fault (2D array with dynamic memory allocation) in C

4 Answers 4

7 Comments

1 Comment

Comments

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

7 Comments

1 Comment

Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related