CODEWITHSUNDEEP
csingly-linked-list
smuzoen
smuzoen

Reputation: 309

Create Fixed Size Linked List in C

I have created a linked list in C that is used to store data which is then modified as required. In creating the linked list I have used the following

struct car_elements
{
  char car_rego[7];
  double time_parked;
  struct car_elements *next;
};

typedef struct car_elements car;

/* Defined as global variable to hold linked list */
car *head = NULL;

car *SetupCars()
{
  car *ptr = head;
  car *new_car = NULL;

  new_car = (car*) malloc(sizeof(car));
  if (!new_car)
  {
    printf("\nUnable to allocate memory!\n");
    exit(1);
  }

  strcpy(new_car->car_rego, "empty");
  new_car->time_parked = time(NULL);
  new_car->next = NULL;

  if (ptr == NULL)
  {
    return (new_car);
  }
  else
  {
    while (ptr->next)
    {
      ptr = ptr->next;
    }
    ptr->next = new_car;
    return (head);
  }
}

From main I call the following to create the linked list

for(int i = 0; i<TOTAL_CARS; i++) {
   head = SetupCars(head);       
}

The problem is that now I have a memory leak - Is there a better way to create a fixed size linked list. At the end of the program running I can

free(head);

However I cannot call within SetupCars method

free(new_car);

I could create new_car as a global variable I guess and free it at the end of the program but I cannot help but feel there is a better way to do it. I don't think global variables are evil if used properly however I would appreciate some advice.

Upvotes: 1

Views: 2256

Answers (3)

James Chong
James Chong

Reputation: 2033

Keep car *head as a global var. For SetupCars:

void SetupCars() /* void will do, unless you want a copy of the new "car" */
{
  car *new_car = NULL;
  new_car = malloc(sizeof *new_car); /* don't need to cast return value of malloc */

  /* do checks and setup new_car... */

  if (head == NULL) /* first element */
  {
     head = new_car;
  }
  else /* easier to add new_car as the FIRST element instead of last */
  { 
     new_car->next = head;
     head = new_car;
  }
}

From main you create the linked list the same way:

for(int i = 0; i<TOTAL_CARS; i++) {
   SetupCars(); /* without any arguments */
}

Then at the end, you loop through the list and free the objects. As Manoj Pandey posted in his answer:

car *tofree;
car *ptr = head;
while(ptr) {
    tofree = ptr;
    ptr = ptr->next;
    free(tofree);
}

Upvotes: 1

Basile Starynkevitch
Basile Starynkevitch

Reputation: 1

You need a function to free the entire list, like:

void free_cars(car*p)  {
   while (p != NULL) {
      car* nextp = p->next;
      free (p);
      p = nextp;
   }
}

So you would call

free_cars (head);
head = NULL;

Perhaps even by having a macro

#define DELETE_CARS(CarsVar) do { \
   free_cars(CarsVar); CarsVar = NULL; } while(0)

then just write DELETE_CARS(head); later in your code.

And indeed, manual memory management is painful, you need to avoid memory leaks. Tools like valgrind can be helpful. And you could consider instead to use Boehm's garbage collector, so use GC_MALLOC instead of malloc and don't bother freeing memory.... Read more about garbage collection.

Upvotes: 3

Manoj Pandey
Manoj Pandey

Reputation: 4666

WHy not just free it at the end? SOmething like this:

car *tofree;
car *ptr = head;
while(ptr) {
    tofree = ptr;
    ptr = ptr->next;
    free(tofree);
}

Upvotes: 4

Related Questions