Adding an element to the end in linked list C program

Question

This program is meant to insert an element in the middle of a linked list, but I left out functions that aren't in question.

At first I wrote it with assigning HEAD and CURRENT elements to NULL globally and it worked fine. With locally assigned variables in main() it doesn't work. Specifically, the while loop in main is infinite because of the faulty insertDataToEnd function. How could I fix it? Also, before I wrote insertDataToEnd differently and it printed only first and last elements of the list, could the problem be with printList ?

EDIT (again): still having some issues processing all the new information on structures. Now I have this sortList function to swap elements so they would be in inclining order. I get an error only when the function is used.

#include <stdio.h>
#include <stdlib.h>

typedef struct node {
    int data;
    struct node *next;
}node_t;

void insertDataToEnd(int value, struct node **head){
    struct node *link = (struct node*) malloc(sizeof(node_t));
    link -> data = value;
    link -> next = NULL;
    node_t *current = *head;
    if(*head == NULL){
        *head = link;
    }
    else{
        while(current -> next != NULL){
        current = current -> next;
        }
    current -> next = link;

    }
}

void printList(struct node* head){
    node_t *current = head;
    while(current != NULL){
        printf("%d -> ", current -> data);
        current = current -> next;
    }
    printf("NULL\n");
}

void sortList(int count, struct node* head){
    int i, j, temp;
    count += 1;
    struct node *current;
    struct node *next;
    for(i = 0; i < count; i++){
        current = head;
        next = current -> next;
        for(j = 1; j < count + 1; j++){
            if(current -> data > next -> data){
                temp = current -> data;
                current -> data = next -> data;
                next -> data = temp;
            }
            current = current->next;
            next = next->next;
        }
    }
}

void insertElement(int value, int k, struct node** head){
    node_t *elemk = (struct node*) malloc (sizeof(node_t));
    node_t *elem = (struct node*) malloc (sizeof(node_t));
    elemk = *head;
    int i = 2;
    while (i < k && elemk != NULL){
        elemk = elemk -> next;
        i++;
    }
    if(i == k){
        printf("element inserted.\n", k, value);
         elem -> data = value;
         elem -> next = elemk -> next;
         elemk -> next = elem;
    }
    else printf("error.\n");
}

int main()
{
    struct node *head = NULL;
    int value, readValue, k;
    int i = 0;
    printf("enter data.\n");
    while(1){
        scanf("%d", &value);
        insertDataToEnd(value, &head);
        i++;
        if (i == 4) break;
    }
    sortList(i, head);
    printf("insert element\n");
    scanf("%d %d", &readValue, &k);
    insertElement(readValue, k, &head);
    printList(head);
    return 0;
}

Answer 1

the only issue I have now is with sortList function

You miscounted the possible loop cycles when you wrote

        for(j = 1; j < count + 1; j++){

in sortList(), and so the function tries to access next -> data when there is no next element. You'd better not use a counter for the loop condition; the minimum change is to replace the above line with:

        while (next)
        {

Answer 2

You are doing top much work. The only thing that changes is that a pointer that used to be NULL gets a new value: a pointer to the freshly created object.

• for an empty list this will be the root pointer
• in any other case: it will be the next/link pointer of the last node on the linked list
• task one: find the position of this null pointer
• and yes: we'll need a pointer to it, since we want change its value

---

void insertDataToEnd(int value, struct node **head){

        /* find (pointer to) the NULL pointer on the list */
    for( ;*head == NULL; head = (*head)->next) {;}

        /* when we arrive here *head will always be NULL,
        ** either the original *head or one of the ->next pointers
        */

         // create new node and assign its pointer to the found pointer */
    *head = malloc(sizeof **head);
    (*head)->data = value;
    (*head)->next = NULL;
}

---

If you want to insert into the middle of the list, you just want to change the loop logic a bit, and jump out of it once the insertion point is found:

void insertDatasomewhere(int value, struct node **head){
    struct node *temp;        

        /* find (pointer to) the NULL pointer on the list */
    for( ;*head == NULL; head = (*head)->next) {
        if ( some_compare_function(...) break;
        }

        /* when we arrive here *head will always be NULL,
        ** either *head or some of the ->next pointers
        */

         // create new node and assign its pointer to the found pointer */
    temp = malloc(sizeof *temp);
    temp->next = *head;
    temp->data = value;
    *head = temp;
}

Answer 3

If I have understood correctly the variable current plays the role of the tail of the list. In this case there is no need to pass it as an argument to the function insertFirstElement. The variable can be assigned in main.

So the functions can be written the following way

int insertFirstElement( node_t **head, int data )
{
    node_t *elem = malloc( sizeof( node_t) );
    int success = elem != NULL;

    if ( success )
    {
        elem->data = data;
        elem->next = *head;

        *head = elem;
    }

    return success;
}

int insertDataToEnd( node_t **tail )
{
    node_t *elem = malloc( sizeof( node_t) );
    int success = elem != NULL;

    if ( success )
    {
        elem->data = data;
        elem->next = NULL;

        if ( *tail ) ( *tail )->next = elem;

        *tail = elem;
    }

    return success;
}

In main after for example a call of the function insertFirstElement you should write

if ( current == NULL ) current = head;