C code involving malloc for dynamic allocation of 2D array crashes after compiling

Question

I am trying to allocate the row dimension of an array (a) to the size of a user input value. Here is the code I have:

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

void FreeArray(int **arry, int N);

int main() {
    int **a;
    int **b;
    int N_a;
    int N;
    if (a != NULL) {
        FreeArray(a, N);
    }
    if (b != NULL) {
        FreeArray(b, N);
    }
    do {
        printf("Enter the size of the NxN array:\n");
        scanf("%d", &N);
        if (N < 2) {
            printf("Error, enter correct size.\n");
        }
    } while (N < 2);
    N_a = N;
    int errorInAlloc = 0;
    a = (int**)malloc(N_a * sizeof(int*));
    if (a != NULL) {
        errorInAlloc = 1;
    }
    return 0;
}

void FreeArray(int **arr, int N) {
    int i;
    if (arr == NULL)
        return;
    if (arr != NULL) {
        for (i = 0; i < N; i++) {
            if (arr[i] != NULL)
                free(arr[i]);
        }
        free(arr);
    }
}

The FreeArray function was provided to us, so I know it is correct. There is other code included in this, but I omitted is since it is just a menu. When I comment out a = (int**) malloc(N_a * sizeof(int*)); the program compiles and runs without crashing or problems. When I include this line, the program compiles, but when I put a number greater than or equal to 2, it crashes. I have also tried a = malloc(N_a * sizeof(int*)); and a = (int**)malloc(N * sizeof(int*)); AND a = malloc(N * sizeof(int*)); but all do the same thing.

I have been coding and running the program in Code::Blocks, but even when I compile it through the command prompt, it still crashes.

I have a hard time with arrays on a good day, so dynamically allocating arrays is so confusing to me, any help is greatly appreciated!

Answer 1

You have multiple errors in your program:

• variables a and b are uninitialized, comparing them to NULL is meaningless and passing them to FreeArray has undefined behavior.

• given how the FreeArray function is written, allocation must be performed in 2 steps:

  • allocate an array of of pointers, which you do.
  • initialize each element of the array with the address of an allocated array of int. which you don't.

Here is how a function AllocArray would be written:

int **AllocArray(int N) {
    int i;
    int **arr = calloc(sizeof(*arr), N);
    if (arr != NULL) {
        for (i = 0; i < N; i++) {
            arr[i] = calloc(sizeof(*arr[i]), N);
            if (arr[i] == NULL) {
                /* allocation failed: free allocated portion and return NULL */
                while (i-- > 0) {
                    free(arr[i]);
                }
                free(arr);
                return NULL;
            }
        }
    }
    return arr;
}

Note that the FreeArray function can be simplified this way:

void FreeArray(int **arr, int N) {
    int i;
    if (arr != NULL) {
        for (i = 0; i < N; i++) {
            free(arr[i]);
        }
        free(arr);
    }
}

It is OK to pass a null pointer to free(), and a single test on arr is sufficient.

Answer 2

First problem is that you free an unallocated pointer:

 if(a != NULL)
 {
     FreeArray(a, N);
 }

You've never allocated memory for a, so when you try to access to a position of the vector, it generates a segmetation fault (same occurs with b). If you want to make sure that your pointer is NULL, just use a = NULL;. This will fix your problem.

There is another logical problem:

if(a != NULL)
{
    errorInAlloc = 1;
}

You return an error in allocation if a differs from NULL. You should return an error if a is NULL wich means that an error occurs in the memory allocation.

Answer 3

Here's what gcc reported when your code was compiled with more warnings.

gcc -o xx xx.c -Wall -Wextra -pedantic -std=c99 -O2 -Wmissing-declarations -Wmissing-prototypes -Wconversion
xx.c: In function ‘main’:
xx.c:26:26: warning: conversion to ‘long unsigned int’ from ‘int’ may change the sign of the result [-Wsign-conversion]
a = (int **) malloc(N_a * sizeof(int *));
                      ^
xx.c:25:6: warning: variable ‘errorInAlloc’ set but not used [-Wunused-but-set-variable]
int errorInAlloc = 0;
  ^
xx.c:10:5: warning: ‘a’ is used uninitialized in this function [-Wuninitialized]
if (a != NULL) {
 ^
xx.c:13:5: warning: ‘b’ is used uninitialized in this function [-Wuninitialized]
if (b != NULL) {
 ^
xx.c:11:3: warning: ‘N’ may be used uninitialized in this function [-Wmaybe-uninitialized]
FreeArray(a, N);
^

The formatting is a little bit off. The point is crystal clear: Let your compiler help you locate errors.