Linked List implementation and manipulations in C

While trying to learn some more C, I decided to implement a linked list and perform a few operations on it.

The first thing I wanted to do was read from a .csv of numbers (ex: 1,2,3,...) and store the numbers in a Linked List.

Once I have the linked list, I want to reverse the linked list and then I want the user to be prompted with the number to remove from the linked list.

Upon receiving the value, the removal operation will search for the value within the linked list and remove it.

After removal, the contents of the linked list will be printed out.

Can somebody look at what I have and give me some pointers on what I did wrong and how I can make it better? One thing I am rather worried about is how I am handling memory allocation and what I am doing is causing any memory leaks.

```
#include
#include

//Defining the linked list struct
typedef struct node{
int data;
struct node* next;
} LList;

//Method to print out the values in a linked list
void printList(LList * head){
LList* current = head;
while(current != NULL){
printf("%d -> ",current->data);
current = current->next;
}
printf(" NULL\n");
}

//Method to reverse a linked list
LList reverse(LList head,LList * reverseHead){
LList * current = head;
while(current != NULL){
if(reverseHead == NULL){
LList * temp = malloc(sizeof(LList));
temp -> data = current -> data;
temp -> next = NULL;
current = current -> next;
reverseHead = temp;
}else{
LList * temp = malloc(sizeof(LList));
temp -> data = current -> data;
current = current -> next;
temp -> next = reverseHead;
reverseHead = temp;
}
}
return reverseHead;
}

//Method to remove an element from the list
LList removeFromList(LList head, int elem){
LList * current = head->next;
LList * prev = head;
LList * modifiedList = head;
while(curren

I have 5 Suggestions.

• No need to pass the reverseHead in LList reverse(LList head,LList * reverseHead) method

Change that to:

LList* reverse(LList * head)

• There is memory leaks in your code. When you are removing an entry from the linked list, you need to release the memory associated with that node.

• I won't prefer multiple return statements from a method

• There are a lot of wild pointers is your code. wild pointers are dangerous, so whenever you declare a pointer, initialize it (At-least initialize it with NULL)

• For the current purpose I don't think you need this statement fgets(buf, sizeof buf, stdin). Instead, you can use gets(buf).

You can change your removeFromList method like:

LList* removeFromList(LList *head, int elem)
{
    LList * current      = head->next;
    LList * prev         = head;
    LList * modifiedList = head;
    LList * deleteNode   = NULL;
    int    flag          = 1;
    while((prev != NULL) && flag)
    {
        if(head -> data == elem)
        {
            if(head->next != NULL)
            {
                head -> data = head -> next -> data;
                deleteNode = head->next;
                head -> next = head -> next -> next;
            }
            else
            {
                deleteNode   = head;
                modifiedList = NULL;
            }
            flag = 0;
        }
        else if(current-> data == elem)
        {
            deleteNode = prev->next;
            prev -> next = deleteNode -> next;
            flag = 0;
        }
        else
        {
            prev = prev -> next;
        }
    }
    if (flag)
    {
        printf("\nEntry %d not found\n",elem);
    }
    free(deleteNode);
    deleteNode = NULL;
    return modifiedList;
}

And change the method calls like:

head = removeFromList(head,listEntry);

StackExchange Code Review Q#44035, answer score: 5