Dynamic array of structs in C

Based on some code on internet, I implemented a dynamic array of structures in C.

I am really interested in some feedback on this. Maybe there are some places where it can cause memory leaks or other pitfalls? (I am also concerned if there can be situation that I free a pointer twice in my case, and how to avoid that)

```
#include "stdafx.h"
#include
#include
#include

typedef struct
{
int ID;
char * name;
} Student;

// array of structs
typedef struct
{
Student *array;
size_t used;
size_t size;
} Array;

void initArray(Array *a, size_t initialSize)
{
// Allocate initial space
a->array = (Student )malloc(initialSize sizeof(Student));

a->used = 0; // no elements used
a->size = initialSize; // available nr of elements

// Initialize all values of the array to 0
for(unsigned int i = 0; iarray[i],0,sizeof(Student));
}
}

// Add element to array
void insertArray(Array *a, Student element)
{
if (a->used == a->size)
{
a->size *= 2;
a->array = (Student )realloc(a->array, a->size sizeof(Student));
}

// Copy name
a->array[a->used].name = (char*)malloc(strlen(element.name) + 1);
strcpy(a->array[a->used].name, element.name);

// Copy ID
a->array[a->used].ID=element.ID;

a->used++;
}

void freeArray(Array *a)
{
// Free all name variables of each array element first
for(int i=0; iused; i++)
{
free(a->array[0].name);
a->array[0].name=NULL;
}

// Now free the array
free(a->array);
a->array = NULL;

a->used = 0;
a->size = 0;
}

int main(int argc, const char * argv[])
{

Array a;
Student x,y,z;

x.ID = 20;
x.name=(char*)malloc(strlen("stud1") + 1);
strcpy(x.name,"stud1");

y.ID = 30;
y.name=(char*)malloc(strlen("student2") + 1);
strcpy(y.name,"student2");

z.ID = 40;
z.name=(char*)malloc(strlen("student3") + 1);
strcpy(z.name,"student3");

// Init array
ini

I am also concerned if there can be situation that I free a pointer twice in my case

... and ...


Yes just I am more interested in cases when/if there can be for example freeing unallocated memory or freeing already freed memory ...

After a quick inspection it doesn't appear that you free memory more than once:

• free statements are only in the freeArray and main methods

• Each free(a->array[0].name); is different because each name is allocated using its own malloc

• free(a->array) is only called once

• freeArray is only called once

• free(x.name); doesn't free the same memory as free(a->array[0].name); because insertArray allocates new memory for each name

and how to avoid that

Something which can help (though not guarantee) is to assign NULL to the pointer after you pass it to free.

• It can help, because calling free on a previously-nulled pointer will harmlessly do nothing

• It's not a guarantee, because you might have more than one pointer pointing to the same memory

dmcr_code's comment below points out a bug. You wrote,

for(int i=0; iused; i++)
{
    free(a->array[0].name);
    a->array[0].name=NULL;
}

This should be,

for(int i=0; iused; i++)
{
    free(a->array[i].name);
    a->array[i].name=NULL;
}

Because you set a->array[0].name=NULL; after freeing it, you don't free it twice.

But, you did fail to free the memory associated with a->array[i].name for values of i larger than 0.


But then how do I protect against that - when array[i].name can contain random value and I try to free it?

To protect yourself:

• Either, don't let it contain a random value (e.g. ensure that it's either a valid pointer, or zero)

• Or, don't use it (e.g. ensure that your a->used logic is correct so that you don't touch elements which you haven't used/initialized).

is memset in the initArray method fine for that?

memset is good:

• You could use calloc instead of malloc to avoid having to use memset as well

• You could use memset on the whole array at once instead of using memset on each element of the array

memset in initArray isn't enough. It's enough to begin with, but there's a realloc in insertArray. So to be good enough, you'd also need to use memset after realloc (to memset the as-yet-unused end of the newly-reallocated array; without using memset on the beginning of the reallocated array, which already contains valid/initialized/used elements).


the only unclear part that remains from your response is how to memset realloced array

Your current code in initArray says,

// Initialize all values of the array to 0
for(unsigned int i = 0; iarray[i],0,sizeof(Student));
}

Another way to do that would be:

// Initialize all elements of the array at once: they are contiguous
memset(&a->array[0], 0, sizeof(Student) * initialSize);

The memset statement to add to insertArray would be:

if (a->used == a->size)
{
    a->size *= 2;
    a->array = (Student *)realloc(a->array, a->size * sizeof(Student));
    // Initialize the last/new elements of the reallocated array
    for(unsigned int i = a->used; isize; i++)
    {
        memset(&a->array[i],0,sizeof(Student));
    }
}

Or:

if (a->used == a->size)
{
    a->size *= 2;
    a->array = (Student *)realloc(a->array, a->size * sizeof(Student));
    // Initialize the last/new elements of the reallocated array
    memset(&a->array[a->used],0,sizeof(Student) * (a->size - a->used));
}

and this comment: "It's not a guarantee, because you might have more than one pointer pointing to the same memory " would be nice if you can address that too

This is safe:

void* foo = malloc(10);
free(foo);
// protect against freeing twice
foo = NULL;
// this is useless and strange, but harmless
free(foo);

This is not safe:

void* foo = malloc(10);
void* bar = foo;
free(foo);
// protect against freeing twice
foo = NULL;
// this is useless and strange, but harmless
free(foo);
// but this is dangerous, illegal, undefined, etc.
// because bar is now pointing to memory that has already been freed
free(bar);

StackExchange Code Review Q#44649, answer score: 8