Am i using malloc properly?

Question

Good day!

I need to use malloc in creating a student list system.... In order to be efficient, our professor asked us to use it on a struct so i created a struct as follows:

struct student {
       char studentID[6];
       char name[31];
       char course [6];
};
struct student *array[30];

everytime i add a record, that is when i use malloc...

array[recordCtr]=(struct student*)malloc(sizeof(struct student));
recordCtr++;

then i free it like this.

 for(i = 0; i < recordCtr; i++){
       free(array[i]);
  } 

Am i using malloc properly??? what is the effect if i free it like this instead of the loop above.

free(array);

Thanks in advance. Your opinion will be highly appreciated.

Answer 1

You are doing fine.

free(array); would be undefined behavior because array itself was not allocated via malloc therefore you can't free it and don't need to - the memory will be managed by the compiler.

Answer 2

A good tip is to always do:

type *something;
something = malloc(n * sizeof(*something));

This is because, if you change the type of something, you don't have to change all sorts of other code. And sizeof is really a compiler operation here, it won't turn into anything different at runtime.

Also, don't cast the void* pointer returned by malloc, there's no reason to do so in C and it just further ties your code together.

So in your case, don't do:

(struct student*)malloc(sizeof(struct student));

but

malloc(sizeof(**array));

Answer 3

There is nothing illegal about the way you are using malloc but this isn't a list, it's an array of pointers.

To use a list you do not fix the size in advance and have a pointer to the next element. You can either make this intrusive of non-intrusive.

For an intrusive list you put struct student * next in the declaration of student. For a non-intrusive list you create another struct student_list_node which contains an instance of struct student and a pointer struct student_list_node * next;

This is an exacmple of the non-intrusive version:

struct student_list_node
{
  struct student data;
  struct student_list_node * next;
};

struct student_list_node * head;
struct student_list_node * tail;

struct student_list_node * addStudentToTail()
{
   struct student_list_node * newnode = (struct student_list_node *)(malloc( sizeof(struct student_list_node ) );
   /* check malloc did not fail or use a checking vesrion of malloc */
   if( !tail )
   {
      head = tail = newnode;
   }
   else
   {
      tail->next = newnode;
      tail = newnode;
   }
   return newnode; // which is also "tail"
}

int main()
{
   struct student_list_node * node = addStudentToTail();
   struct student * pstud = &node->data;
   /* write to pstud student details */
}

If you do really want to use an array, you might want to make it an array of student rather than student * in which case you can use calloc rather than malloc

struct student * array = (struct student *)calloc( 30, sizeof( student ) );

Then using free(array) would be the correct way to dispose of it. You also have the option of allocating more if you need it later with realloc. (Be careful with this one: you must keep a copy of your original pointer until you know realloc succeeds).

Answer 4

The array itself isn't allocated on the heap. Assuming it's a global variable, it is allocated in global memory at program startup and doesn't need to be freed. Calling free on it will probably corrupt your program.

Your current solution is correct.

Answer 5

What you're doing is correct.

You can think of *array[30] as an array of 30 pointers When you are allocating memory for each of those pointers, you'd also need to call free() on each of them.

Answer 6

Yes, you are using it correctly. There are better ways to organize the storage than this, but this will work. At least until you need more than 30 students...

Note that you must call free() with each pointer that is returned by malloc(). That means that your loop over the array of pointers is the correct approach for your chosen architecture.

Your attempt to call free on the array itself will not work. It invokes Undefined Behavior because you are passing a pointer (to the base of the array itself) to free() that did not come from a call to malloc().

Answer 7

Looks fine.

You could (if it fits your problem) allocate space for all 30 structs in one go

 struct student *array = (struct student *)malloc(30*sizeof(struct student));

whhen you want to dispose of the space, you can then do

free(array)

Answer 8

What you have will work just fine. As others have mentioned, you've created an array of pointers on the stack and need to malloc and free each of them individually as you are doing.

However, you don't have to use malloc and free one struct at a time, you can do something like this:

int arraySize = 30;
student * ourArray = (student*)malloc(sizeof(student) * arraySize);

and a single free on the pointer will take care of it. with this pointer, you can still use the bracket notation and the compiler will understand that it's a typed pointer and behave appropriately, giving you essentially the same thing. Which method you use depends on if you need your array a dynamic size or not as well as personal preference.

Hope that helps some.

Answer 9

Initialize your array of pointer to struct student with NULL values

for(i = 0; i < recordCtr; i++){
       array[i] = NULL;
  } 

Free memory if array[i] is not NULL

for(i = 0; i < recordCtr; i++){
       if(NULL != array[i])
       {
           free(array[i]);
       }
  } 

Answer 10

There's simple rule: Every malloc() should be paired with free() with pointer returned by malloc. Not less, not more.