Question

I'm creating variables like this:

    pthread_t *thread;
    struct thread_data *data;

    //allocate space for threads and their data
    thread = malloc(num_threads*sizeof(*thread));
    data = malloc(num_threads*sizeof(*data));

And after I'm done with them I'm trying to free the memory as follows:

    //uuu... there is no garbage collector :P
    for (unsigned long i=0;i<num_threads;i++){
        free(&thread[i]);
        free(&data[i]);
    }

However, I'm getting an Invalid pointer error. I'm a bit new to c so any guidance is appreciated.

P.S.: This is how struct looks like.

struct thread_data{
    int base,           //base term from which computation will start
        num_terms;      //numer of terms to compute
    double result;
};
Was it helpful?

Solution

Each call to malloc() shall match exactly one call to free().

So modifiy the code to free the memory allocated in the question to look like this:

    free(thread);
    free(data);

Update:

Please note that exactly the values as returned by malloc() need to be passed to free().

OTHER TIPS

You simply need to do

free(thread);
free(data);

The free function frees the memory block pointed to by its argument. The information about the size of the block is stored internally and you don't need to free each element of the dynamically allocated array.

Thus, When the return value of malloc is passed to free, it frees the entire memory block allocated.

In this case it sees that you are intending to delete all the threads and data elements in one go, so the accepted answer stands; but if you did want to deallocate individual thread and data items at different times, you have to have allocated then individually. For example:

pthread_t** thread;  // Dynamic array of pointers to pthread_t

// Allocate pthread_t* array
threads = malloc(num_thread * sizeof(*thread));

// Allocate pthread_t elements
for( i = 0; i < num_threads; i++ )
{
    thread[i] = malloc( sizeof(*thread[i]) ) ;
}

// Similarly for data...

The the clean-up is more subtle (and perhaps error prone):

free( thread[i] ) ;
thread[i] = 0 ;  // Set pointer to null so element cannot be used after deletion

And if all thread elements are deleted, the thread pointer array can be deleted:

free( thread ) ;

You could have a clean-up all operation:

for( i = 0; i < num_threads; i++ )
{
    free( thread[i] ) ;
    thread[i] = 0 ;
}
free( thread ) ;
thread = 0 ;

Freeing a null pointer is safe, so there is no need to test if an element has previously been deleted, but you would need to have tested it before using it of course.

Note that dereferencing a null pointer is specifically trapped as a run-time error, while dereferencing a stale pointer will have non-deterministic results, and will often remain unnoticed in your code until the day you make some unrelated change causes the code to crash inexplicably - so it is good practice to set the pointer to 0 after deallocation - it will allow some bugs to be detected earlier and more easily fixed.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top