0

I have a pointer to a struct and one of the objects in the struct is a int **. The double pointer is used to dynamically allocate memory for a 2d array. I am having trouble figuring out how to free the memory for this array. Any ideas?

struct time_data {
    int *week;
    int *sec;
    int **date;
};
typedef struct time_data time_data;

time_data *getTime(time_data *timeptr, int rows, int cols) {
    int i = 0;
    time_data time;

    // allocate memory for time.date field
    time.date = (int **)malloc(rows*(sizeof(int *))); // allocate rows
    if(time.date == NULL)
        printf("Out of memory\n");
    for(i=0; i<rows; i++) {
        time.date[i] = (int *)malloc(cols*sizeof(int));
        if(time.date[i] == NULL)
            printf("Out of memory\n");
    }
    timeptr = &time;
    return timeptr;
}

int main(int argc, const char * argv[]) {
    time_data *time = NULL;
    int rows = 43200, cols = 6;
    int i;
    time = getTime(time, rows, cols);

    for(i=0; i<rows; i++)
        free(time->date[i]); // problem here
    free(time->date);

}

Modified Version (in case anyone else has similar issue)

    struct time_data {
    int *week;
    int *sec;
    int **date;
};
typedef struct time_data time_data;

time_data *getTime(int rows, int cols) {
    int i = 0;
    time_data *time = malloc(sizeof(*time));

    // allocate memory for time.date field
    time->date = (int **)malloc(rows*(sizeof(int *))); // allocate rows
    if(time->date == NULL)
        printf("Out of memory\n");

    for(i=0; i<rows; i++) {
        time->date[i] = (int *)malloc(cols*sizeof(int));
        if(time->date[i] == NULL)
            printf("Out of memory\n");
    }
    return time;
}

int main(int argc, const char * argv[]) {
    time_data *time = NULL;
    int rows = 43200, cols = 6;
    int i;
    time = getTime(rows, cols);

    for(i=0; i<rows; i++)
        free(time->date[i]); // problem here
    free(time->date);
return 0;
}
biononic
  • 95
  • 6
  • [Don't cast malloc](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – Barmar Jan 29 '15 at 02:10
  • What ideas? you are doing it right. Just remove the cast from malloc. And also, `printf("Out of memory\n");` is not enough, you shold abort and not dereferene the pointer. – Iharob Al Asimi Jan 29 '15 at 02:11
  • 1
    You're freeing it correctly. But you're not allocating it correctly. `getTime` is returning the address of a local variable, which is not valid. – Barmar Jan 29 '15 at 02:12
  • Why do I want to remove the cast from malloc? – biononic Jan 29 '15 at 04:56

1 Answers1

2

Your freeing is ok, but you have a severe error

timeptr = &time;
return timeptr;

you are returning the address of a local variable.

The local variable is allocated in the stack frame of the function, and once the function returns, the data will no longer exist.

You should use malloc for that too

timeptr = malloc(sizeof(*timeptr));

and also you must return an int from main()

Fiddling Bits
  • 8,712
  • 3
  • 28
  • 46
Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97
  • I changed the code to malloc the memory for the pointer but I am still getting the same error when I try to free the memory in main. Thread 1: EXC_BAD_ACCESS(code=1,address 0x0). If I remove the for loop everything runs fine. Correct me if I'm wrong, put I think I'm returning the address to the 2D array that I allocated. timeptr is just used to store that address until it can be returned. Do I need to malloc memory for the entire struct time, instead of doing it for a particular field? – biononic Jan 29 '15 at 04:22
  • Thanks for pointing me in the right direction. I changed the input arguments for getTime and got rid the time_data time declaration and just used the pointer to the struct. Had to change all references to fields to time->date format. Seems to be working now – biononic Jan 29 '15 at 04:49