Allocation of memory, segmentation error in C

Question

I have this problem I need to solve. I am given a program which is supposed to allocate memory to create an array of doubles, but something is wrong with it. Basically, main and the function create_array are broken, and I need to fix it. The solution I came up with comes up with a segmentation error and i am not sure why.

First off I decided to delete the "free(array)" in main b/c array is a pointer which does not exist in heap from my understanding. I tried changing the function "create_array" to the following

void create_array(double *x, unsigned long n) {

   double *d = malloc((int)n *sizeof(double));

    if(d == NULL){
        printf("Sorry Memory Not Available. Program Terminated.\n");
        exit(1);

    }

    x=d;
    free(d);

}

Here is what I was provided with:

int main(void) {
    printf("\nProgram started...\n");
    double *array = NULL;
    int n = 20;
    create_array(array, n);

    if( array != NULL) {
        populate_array(array, n);

        // displays half of the values of the array
        for(int i = 0; i < n/2; i++){
            printf("%f\n", *array++);
        }

        // According to C standard, the program's behaviour, after the following
        // call to the function free is considered "Undefined" and needs to be
        // fixed.
        free(array);
    }

    printf("Program terminated...\n");
    return 0;
}

// THE FOLLOWING FUNCTION IS NOT PROPERLY DESINGED AND NEEDS TO BE FIXED
void create_array(double *x, unsigned long n) {
    x = malloc(n *sizeof(double));
    if(x == NULL){
        printf("Sorry Memory Not Available. Program Terminated.\n");
        exit(1);
    }
}

void populate_array(double *array, int n) {
    int i;
    for(i = 0; i < n; i++)
        array[i] = (i + 1) * 100;
}

When I run the code with my edit, i get segmentation error. When I run the code with no changes, it simply outputs "program started \n program terminated".

Answer 1

There are several errors in create_array. x is a local variable, so it will not change. Also, you free the memory after initialization, which in practice makes the whole function useless. Free should be done after you're done with the memory.

Here is a corrected version

void create_array(double **x, unsigned long n) {
    double *d = malloc((int)n *sizeof(double));

    if(d == NULL){
        printf("Sorry Memory Not Available. Program Terminated.\n");
        exit(1);
    }

    *x=d;
}

And then you call it like this:

double *p;
create_array(&p, size);
/* Do something */
free(p);

But, IMHO it would be better to do like this:

double *create_array(size_t n) {
    double *d = malloc(n*sizeof(*d));

    if(d == NULL){
        fprintf(stderr, "Sorry Memory Not Available. Program Terminated.\n");
        exit(1);
    }

    return d;
}

Changes:

• Returns the pointer instead of using an output parameter
• Use size_t instead of unsigned long
• Print errors to stderr insteadd of stdio
• Use malloc(size *sizeof(<var>)) instead of malloc(size * sizeof(<type>))

And here is the call:

double *p = create_array(&p, size);
/* Do something */
free(p);

Answer 2

The problem is, in both code and snippet provided, the variable assigned by malloc's return value lives until the function returns. So you need to use pointer-to-pointer. Moreover, in the first snippet, if you free the area allotted already, you can't use the space anymore.

void create_array(double **x, unsigned long n) {
    *x = malloc(n *sizeof(double));
    if(*x == NULL){
        printf("Sorry Memory Not Available. Program Terminated.\n");
        exit(1);
    }
}

In main, you can call it as create_array(&array, n);.