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;
}

Answer 1

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

Answer 2

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.

Answer 3

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

Answer 4

#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;
}