Creating a list using a struct in C

Question

I just got into structs and I decided to create a list using them:

#include<stdio.h>
#include<stdlib.h>
int main(void)
{
 char counter;
 struct list
 {
 int x; 
 struct list *pointer1;
 };
 struct list test;
 struct list *pointer2;
 pointer2=&test;
 for(counter=0;counter<5;counter++)
 {
 pointer2->pointer1=malloc(sizeof(struct list)); 
 pointer2=pointer2->pointer1; 
 } 
}

I was wondering if there is another way to do the exact same thing. Is the use of one pointer only possible? By the way, I am pretty sure I could do the same thing using an array of structs too. Any advice would be really helpful!

Answer 1

This is actually rather unconventional code. The head node of this list, test, resides on the stack. All of the subsequent nodes are allocated from the heap using malloc(). Since you never call free() on the memory returned by malloc(), you have a memory leak. When you eventually try to fix your memory leak, you're very likely to get confused: you need to free() all the nodes except test.

Furthermore, linked lists are usually terminated by a NULL pointer. After this program finishes, though, the tail of the list is an uninitialized chunk of memory from malloc(), which you should consider to return random garbage. (In practice, you are likely to get lucky and receive pre-zeroed memory since your program is running on a virgin heap, but you should not count on that behaviour.)

I suggest that you build the linked list all on the heap. Also, build it backwards, starting from the tail, so that your pointer is always pointing to the head. When you're done, you'll be holding a pointer to the head, which is useful.

#include <stdlib.h>
int main(void)
{
 struct list
 {
 int x; 
 struct list *next;
 };
 /* Create the list */
 struct list *head = NULL;
 /* Just use an int for the counter. A char is just weird. */
 for (int counter = 0; counter < 6; counter++)
 {
 struct list *node = malloc(sizeof(*node));
 node->next = head;
 head = node;
 }
 /* Destroy the list */
 while (head) {
 struct list *node = head;
 head = head->next;
 free(node);
 }
 return 0;
}

• Omer: since we are in code review site, wouldn't you want the int in the for loop to be unsigned?