Dynamic Memory Question

[iAndrewMeyer]

Am I freeing and declaring this 2d array correctly?

int **ratings;

ratings = (int**)malloc(sizeof(int *) * (numCouples*2));
        for(i=0; i<numCouples*2*numCouples;i++)
                ratings[i] = (int*)malloc(sizeof(int) * numCouples);

        //Data is read into each array position
        for(i=0; i<numCouples*2;i++)
            for(j=0;j<numCouples;j++){
                fscanf(ifp, "%d", &ratings[i][j]);
                //printf("%d",ratings[i][j]);
            }

            for(i=0;i<(numCouples*2);i++)
                free(ratings[i]);
            free(ratings);

Thanks

Drew

[deceptikon]

Your initial allocation loop iterates far too many times. The overall process looks fine though. I'd write it like this:

int rows = numCouples * 2;
int cols = numCouples;
int** ratings = malloc(rows * sizeof *ratings);

// Check if malloc() failed...

for (i = 0; i < rows; ++i)
{
    ratings[i] = malloc(cols * sizeof *ratings[i]);
}

// Populate the data...

for (i = 0; i < rows; ++i)
{
    free(ratings[i];
}

free(ratings);

[dheaven]

I would rather do it like this:

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

#define ROWS 5
#define COLUMNS 4

int main(int argc, char** argv)
{
    int** array = NULL;
    int i = 0;
    /* Memory allocation */
    array = (int**)malloc(sizeof(int) * ROWS);
    if(array == NULL)
    {
        perror("Not enough memory for matrix allocation");
    }
    for(i=0; i<ROWS; i++)
    {
        array[i] = (int*)malloc(sizeof(int) * COLUMNS);
        if(array[i] == NULL)
        {
            perror("Not enough memory for row allocation");
        }
    }
    /* Freeing the memory */
    for(i=0; i<ROWS; i++)
    {
        free(array[i]);
    }
    free(array);
    return 0;
}

[histrungalot]

/* Memory allocation */
array = (int**)malloc(sizeof(int) * ROWS);

Should be

/* Memory allocation */
array = (int**)malloc(sizeof(int*) * ROWS);

sizeof(int) and sizeof(int *) would be different on 64-bit machine.

[Trentacle]

Which is why you should instead use

array = malloc(ROWS * sizeof *array);

Also don't cast the result of malloc(), it suppresses warnings and serves no useful purpose. deceptikon's solution was in every way superior.

[dheaven]

sizeof(int) and sizeof(int *) would be different on 64-bit machine.

True, you need you need to use int*

Also don't cast the result of malloc()

There some C standards (such as MISRA) which recommend casting void*. Also, to my knowledge, this increases the portability of your code. (even if it supresses the missing stdlib.h warning)

[Trentacle]

There some C standards (such as MISRA) which recommend casting void*. Also, to my knowledge, this increases the portability of your code.

There are several legitimate reasons to cast a pointer-to-void, mostly to do with alignment. malloc(), however, is guaranteed to return a pointer of correct alignment for any type, so no alignment adjustment is necessary. Furthermore, in an assignment statement, the C standard requires that type conversion take place as if the RHS were cast to the type of the LHS. Casting the return value of malloc() is not just an extra step -- it's a complete no-op. Literally the only reason to do so is to suppress compiler warnings. Show me an implementation where omitting the cast affects the behavior and I'll show you a language that isn't C.

[Trentacle]

I'd also like to note that MISRA C forbids the use of malloc() altogether. Perhaps for related reasons.

[dheaven]

There are several legitimate reasons to cast a pointer-to-void, mostly to do with alignment. malloc(), however, is guaranteed to return a pointer of correct alignment for any type, so no alignment adjustment is necessary. Furthermore, in an assignment statement, the C standard requires that type conversion take place as if the RHS were cast to the type of the LHS. Casting the return value of malloc() is not just an extra step -- it's a complete no-op. Literally the only reason to do so is to suppress compiler warnings. Show me an implementation where omitting the cast affects the behavior and I'll show you a language that isn't C.

I agree with you, but I didn't said that you need to cast a void pointer, I just said that it increases portability and robustness.

I'd also like to note that MISRA C forbids the use of malloc() altogether. Perhaps for related reasons.

malloc() is not used in embedded systems (especially critical-safety systems such as the ones that are in automotive) because it leads to memory fragmentation and the allocation process can become quite slow if the memory is getting filled.

[Trentacle]

I agree with you, but I didn't said that you need to cast a void pointer, I just said that it increases portability and robustness.

And I literally just explained why, in C, that is not true.

malloc() is not used in embedded systems (especially critical-safety systems such as the ones that are in automotive) because it leads to memory fragmentation and the allocation process can become quite slow if the memory is getting filled.

I was pointing out the fallacy of trying to apply MISRA to code that uses malloc(). It's like calling the cops because someone stole your marijuana.